From eb8f08a182e190111da2a5a7f4dcd457d652f846 Mon Sep 17 00:00:00 2001 From: Andy Dixon Date: Tue, 13 Oct 2026 06:49:13 +0100 Subject: [PATCH] fix(rdp): clipboard only on explicit paste, size check before allocation, bounded device I/O - The local clipboard was offered to the RDP server every time the desktop gained focus, so any server could read the last thing copied (a password, a token) without a paste. It now goes only on Ctrl/Cmd+V or Paste Clipboard. - The server-chosen desktop size is checked before DecodedImage allocates it (65535x65535 is 16 GiB, and allocation failure aborts the whole app), at connect and on reactivation. - Drive reads are clamped to 8 MiB per request (the native backend allocated the server's u32 length up front). - Print jobs get unique files created exclusively (same-second jobs used to truncate each other) and at most 4 may be open at once. --- docs/user-manual.md | 2 +- rust/app/src/desktop_view.rs | 12 +++------- rust/core/src/clients/rdp.rs | 6 +++-- rust/core/src/clients/rdp_devices.rs | 33 ++++++++++++++++++++++++---- 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/docs/user-manual.md b/docs/user-manual.md index a6749a9..513c288 100644 --- a/docs/user-manual.md +++ b/docs/user-manual.md @@ -175,7 +175,7 @@ Nexus Terminal logs on with Network Level Authentication (CredSSP), so your pass RDP servers usually present self-signed certificates, so the certificate is pinned like an SSH host key: its SHA-256 fingerprint is shown the first time and stored in **Known Hosts**, and a changed certificate offers Reject, Accept Once or Accept & Update. The check happens before any credentials are sent. -The viewer works like the VNC one: click the desktop to give it keyboard focus, **Ctrl+Alt+Del** sends the key combination, and text is shared through the clipboard both ways: text you copy on the remote desktop arrives on your clipboard (subject to the remote-clipboard setting), and your clipboard is offered to the remote desktop whenever you click into it, press Ctrl+V or use **Paste Clipboard**. Files and images are not shared. With **Resize With Tab**, the remote desktop follows the tab's size on servers that support it (Windows); otherwise the picture is scaled. +The viewer works like the VNC one: click the desktop to give it keyboard focus, **Ctrl+Alt+Del** sends the key combination, and text is shared through the clipboard both ways: text you copy on the remote desktop arrives on your clipboard (subject to the remote-clipboard setting), and your clipboard is sent to the remote desktop only when you paste (Ctrl+V / Cmd+V on the desktop, or **Paste Clipboard**), so a server can't read what you last copied. Files and images are not shared. With **Resize With Tab**, the remote desktop follows the tab's size on servers that support it (Windows); otherwise the picture is scaled. ### IRC diff --git a/rust/app/src/desktop_view.rs b/rust/app/src/desktop_view.rs index cef9921..68ddf14 100644 --- a/rust/app/src/desktop_view.rs +++ b/rust/app/src/desktop_view.rs @@ -446,15 +446,9 @@ impl DesktopView { resp.request_focus(); } let focused = resp.has_focus(); - // RDP shares the clipboard both ways: offer the local text whenever the desktop gains focus. - if focused - && !self.had_focus - && let Some(Remote::Rdp(c)) = &self.client - && let Ok(text) = arboard::Clipboard::new().and_then(|mut c| c.get_text()) - && !text.is_empty() - { - c.set_clipboard(text); - } + // The local clipboard goes to the remote desktop only on an explicit paste (Ctrl/Cmd+V or + // Paste Clipboard). Offering it whenever the desktop gained focus let any server read + // whatever was last copied (a password, a token) without the user pasting anything. if focused { ui.painter().rect_stroke(img, 0.0, egui::Stroke::new(1.0, Tokens::get(ui.ctx()).accent.gamma_multiply(0.6)), egui::StrokeKind::Outside); ui.memory_mut(|m| m.set_focus_lock_filter(id, egui::EventFilter { tab: true, horizontal_arrows: true, vertical_arrows: true, escape: true })); diff --git a/rust/core/src/clients/rdp.rs b/rust/core/src/clients/rdp.rs index ca94811..ed04f22 100644 --- a/rust/core/src/clients/rdp.rs +++ b/rust/core/src/clients/rdp.rs @@ -259,8 +259,10 @@ impl Client { fn run(&self, mut framed: Framed, result: ConnectionResult, rx: Receiver) -> Result<()> { framed.get_inner_mut().0.sock.set_read_timeout(Some(POLL))?; let size = result.desktop_size; - let mut image = DecodedImage::new(PixelFormat::RgbA32, size.width, size.height); + // Check the server-chosen size before DecodedImage allocates it (65535² is 16 GiB, and + // an allocation failure aborts the whole app rather than failing this session). *self.fb.lock() = Framebuffer::new_checked(size.width, size.height)?; + let mut image = DecodedImage::new(PixelFormat::RgbA32, size.width, size.height); let factory = result.activation_factory; let mut stage = ActiveStageBuilder { static_channels: result.static_channels, @@ -349,8 +351,8 @@ impl Client { if let ConnectionActivationState::Finalized { desktop_size, share_id, enable_server_pointer, pointer_software_rendering } = seq.connection_activation_state() { - image = DecodedImage::new(PixelFormat::RgbA32, desktop_size.width, desktop_size.height); *self.fb.lock() = Framebuffer::new_checked(desktop_size.width, desktop_size.height)?; + image = DecodedImage::new(PixelFormat::RgbA32, desktop_size.width, desktop_size.height); stage.set_fastpath_processor( fast_path::ProcessorBuilder { io_channel_id: seq.io_channel_id(), diff --git a/rust/core/src/clients/rdp_devices.rs b/rust/core/src/clients/rdp_devices.rs index bc2655d..0fe3c6a 100644 --- a/rust/core/src/clients/rdp_devices.rs +++ b/rust/core/src/clients/rdp_devices.rs @@ -25,6 +25,11 @@ use std::path::{Path, PathBuf}; /// A print job may not grow past this (a hostile server could otherwise fill the disk). const MAX_PRINT_JOB: u64 = 256 << 20; +/// Print jobs open at once; each holds a file descriptor. +const MAX_OPEN_PRINT_JOBS: usize = 4; +/// Largest drive read served in one request. The server picks the length (a u32), and the +/// native backend allocates it before reading; Windows clients never ask for more than ~1 MiB. +const MAX_DRIVE_READ: u32 = 8 << 20; /// Called with the file a finished print job was saved to. pub type PrintedFn = std::sync::Arc; @@ -69,9 +74,16 @@ impl Spool { fn handle(&mut self, req: PrinterIoRequest) -> PduResult> { let pdu = match req { PrinterIoRequest::Create(r) => { - let name = format!("Nexus print {}.ps", chrono::Local::now().format("%Y-%m-%d %H.%M.%S")); - let path = self.dir.join(name); - match std::fs::File::create(&path) { + // A fresh file per job: two jobs in the same second used to share (and truncate) + // one file, and an existing file of that name was overwritten. + let stamp = chrono::Local::now().format("%Y-%m-%d %H.%M.%S"); + let path = self.dir.join(format!("Nexus print {stamp} #{}.ps", self.next_id)); + let file = if self.jobs.len() >= MAX_OPEN_PRINT_JOBS { + Err(std::io::Error::other("too many print jobs open")) + } else { + std::fs::OpenOptions::new().write(true).create_new(true).open(&path) + }; + match file { Ok(f) => { let id = self.next_id; self.next_id += 1; @@ -220,7 +232,11 @@ impl ConfinedDrive { impl ConfinedDrive { #[cfg(unix)] - fn handle(&mut self, req: ServerDriveIoRequest) -> PduResult> { + fn handle(&mut self, mut req: ServerDriveIoRequest) -> PduResult> { + if let ServerDriveIoRequest::DeviceReadRequest(r) = &mut req { + // A short read is valid RDPDR; the server asks again for the rest. + r.length = r.length.min(MAX_DRIVE_READ); + } let ok = match &req { ServerDriveIoRequest::ServerCreateDriveRequest(r) => self.allowed(&r.path), ServerDriveIoRequest::ServerDriveQueryDirectoryRequest(r) => r.initial_query == 0 || self.allowed(r.path.trim_end_matches('*')), @@ -283,6 +299,15 @@ mod tests { dev.handle_printer_io_request(write(3, b"x")).unwrap(); assert_eq!(dev.printer.as_ref().unwrap().jobs[&3].2, MAX_PRINT_JOB); assert_eq!(printed.lock().len(), 1); + dev.handle_printer_io_request(close(3)).unwrap(); + // Jobs started in the same second get their own files, and only a few may be open at once. + for _ in 0..(MAX_OPEN_PRINT_JOBS + 3) { + dev.handle_printer_io_request(create()).unwrap(); + } + let jobs = &dev.printer.as_ref().unwrap().jobs; + assert_eq!(jobs.len(), MAX_OPEN_PRINT_JOBS); + let paths: std::collections::HashSet<_> = jobs.values().map(|j| j.1.clone()).collect(); + assert_eq!(paths.len(), MAX_OPEN_PRINT_JOBS); std::fs::remove_dir_all(dir).unwrap(); }