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.
This commit is contained in:
Andy Dixon
2026-10-13 06:49:13 +01:00
parent 7f3704e890
commit eb8f08a182
4 changed files with 37 additions and 16 deletions
+3 -9
View File
@@ -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 }));
+4 -2
View File
@@ -259,8 +259,10 @@ impl Client {
fn run(&self, mut framed: Framed<Tls>, result: ConnectionResult, rx: Receiver<Input>) -> 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(),
+29 -4
View File
@@ -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<dyn Fn(PathBuf) + Send + Sync>;
@@ -69,9 +74,16 @@ impl Spool {
fn handle(&mut self, req: PrinterIoRequest) -> PduResult<Vec<SvcMessage>> {
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<Vec<SvcMessage>> {
fn handle(&mut self, mut req: ServerDriveIoRequest) -> PduResult<Vec<SvcMessage>> {
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();
}