From 6382bd433a66683d29455409e1dbb16776727bbf Mon Sep 17 00:00:00 2001 From: akbash Date: Tue, 21 Jul 2026 17:33:07 +0300 Subject: fix: reject oversized text pastes safely (#1676) refs #1665 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com> --- src/server/client_transport.rs | 299 +++++++++++++++++++++++++++++++++++------ src/server/headless.rs | 89 ++++++++++++ 2 files changed, 349 insertions(+), 39 deletions(-) (limited to 'src/server') diff --git a/src/server/client_transport.rs b/src/server/client_transport.rs index c4e43e0d..743a108f 100644 --- a/src/server/client_transport.rs +++ b/src/server/client_transport.rs @@ -300,6 +300,12 @@ pub(crate) enum ServerEvent { client_id: u64, events: Vec, }, + /// A fully decoded interactive paste exceeded the text-input limit. + ClientPasteRejected { + client_id: u64, + size: usize, + max: usize, + }, /// A client sent local clipboard image bytes to paste into a remote pane. ClientClipboardImage { client_id: u64, @@ -372,22 +378,30 @@ fn parse_client_keybindings( } } -fn input_events_within_limits(events: &[ClientInputEvent]) -> bool { +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum InputEventLimit { + WithinLimits, + TooManyEvents, + PasteTooLarge { size: usize }, +} + +fn input_event_limit(events: &[ClientInputEvent]) -> InputEventLimit { if events.len() > MAX_INPUT_EVENT_BATCH { - return false; + return InputEventLimit::TooManyEvents; } let mut paste_bytes = 0usize; for event in events { if let ClientInputEvent::Paste { text } = event { paste_bytes = paste_bytes.saturating_add(text.len()); - if paste_bytes > MAX_INPUT_PAYLOAD { - return false; - } } } - true + if paste_bytes > MAX_INPUT_PAYLOAD { + InputEventLimit::PasteTooLarge { size: paste_bytes } + } else { + InputEventLimit::WithinLimits + } } #[cfg(windows)] @@ -650,32 +664,60 @@ fn client_read_loop( ClientMessage::Input { data } => { // Validate input size. if data.len() > MAX_INPUT_PAYLOAD { - warn!( - client_id, - size = data.len(), - "oversized input from client, closing" - ); - let _ = server_event_tx - .blocking_send(ServerEvent::ClientDisconnected { client_id }); - break; + if crate::raw_input::is_complete_text_bracketed_paste(&data) { + warn!( + client_id, + size = data.len(), + max = MAX_INPUT_PAYLOAD, + "oversized bracketed paste from client, rejecting" + ); + ServerEvent::ClientPasteRejected { + client_id, + size: data.len(), + max: MAX_INPUT_PAYLOAD, + } + } else { + warn!( + client_id, + size = data.len(), + "oversized input from client, closing" + ); + let _ = server_event_tx + .blocking_send(ServerEvent::ClientDisconnected { client_id }); + break; + } } else { ServerEvent::ClientInput { client_id, data } } } - ClientMessage::InputEvents { events } => { - if !input_events_within_limits(&events) { + ClientMessage::InputEvents { events } => match input_event_limit(&events) { + InputEventLimit::WithinLimits => { + ServerEvent::ClientInputEvents { client_id, events } + } + InputEventLimit::TooManyEvents => { warn!( client_id, count = events.len(), - "oversized input events from client, closing" + "oversized input event batch from client, closing" ); let _ = server_event_tx .blocking_send(ServerEvent::ClientDisconnected { client_id }); break; - } else { - ServerEvent::ClientInputEvents { client_id, events } } - } + InputEventLimit::PasteTooLarge { size } => { + warn!( + client_id, + size, + max = MAX_INPUT_PAYLOAD, + "oversized structured paste from client, rejecting" + ); + ServerEvent::ClientPasteRejected { + client_id, + size, + max: MAX_INPUT_PAYLOAD, + } + } + }, ClientMessage::ObserveTerminal { target } => { ServerEvent::ClientObserveTerminal { client_id, target } } @@ -799,6 +841,29 @@ mod tests { (client, server, TestSocketPath(path)) } + fn recv_server_event(receiver: &mut mpsc::Receiver, context: &str) -> ServerEvent { + let deadline = std::time::Instant::now() + Duration::from_secs(1); + loop { + match receiver.try_recv() { + Ok(event) => return event, + Err(mpsc::error::TryRecvError::Empty) if std::time::Instant::now() < deadline => { + std::thread::sleep(Duration::from_millis(1)); + } + Err(err) => panic!("{context}: {err}"), + } + } + } + + fn bracketed_paste_with_total_len(total_len: usize) -> Vec { + const DELIMITER_BYTES: usize = b"\x1b[200~".len() + b"\x1b[201~".len(); + assert!(total_len >= DELIMITER_BYTES); + let mut data = Vec::with_capacity(total_len); + data.extend_from_slice(b"\x1b[200~"); + data.resize(total_len - b"\x1b[201~".len(), b'x'); + data.extend_from_slice(b"\x1b[201~"); + data + } + fn test_queue_writer() -> (ClientWriter, Arc) { let queue = ClientWriterQueue::new(); ( @@ -1175,7 +1240,7 @@ new_tab = "ctrl+notakey" } #[test] - fn client_read_loop_rejects_oversized_input() { + fn client_read_loop_rejects_oversized_bracketed_paste_without_disconnect() { let (mut client_stream, server_stream, _path) = local_stream_pair("client-read-oversized"); let (server_event_tx, mut server_event_rx) = mpsc::channel(4); let should_quit = Arc::new(AtomicBool::new(false)); @@ -1187,19 +1252,120 @@ new_tab = "ctrl+notakey" protocol::write_message( &mut client_stream, &ClientMessage::Input { - data: vec![b'x'; MAX_INPUT_PAYLOAD + 1], + data: bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD), }, ) - .expect("write oversized input"); + .expect("write maximum-size bracketed paste"); - match server_event_rx - .blocking_recv() - .expect("client disconnected event") - { - ServerEvent::ClientDisconnected { client_id } => assert_eq!(client_id, 7), - other => panic!("expected ClientDisconnected, got {other:?}"), + match recv_server_event(&mut server_event_rx, "maximum-size paste event") { + ServerEvent::ClientInput { client_id, data } => { + assert_eq!(client_id, 7); + assert_eq!(data.len(), MAX_INPUT_PAYLOAD); + } + other => panic!("expected maximum-size ClientInput, got {other:?}"), + } + + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD + 1), + }, + ) + .expect("write oversized bracketed paste"); + + match recv_server_event(&mut server_event_rx, "oversized paste rejection") { + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + assert_eq!(client_id, 7); + assert_eq!(size, MAX_INPUT_PAYLOAD + 1); + assert_eq!(max, MAX_INPUT_PAYLOAD); + } + ServerEvent::ClientDisconnected { .. } => { + panic!("oversized input must be rejected without disconnecting the client") + } + other => panic!("expected ClientPasteRejected, got {other:?}"), } + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: b"still connected".to_vec(), + }, + ) + .expect("write valid input after rejection"); + + match recv_server_event(&mut server_event_rx, "valid input after rejection") { + ServerEvent::ClientInput { client_id, data } => { + assert_eq!(client_id, 7); + assert_eq!(data, b"still connected"); + } + other => panic!("expected ClientInput after rejection, got {other:?}"), + } + + drop(client_stream); + should_quit.store(true, Ordering::Release); + handle + .join() + .expect("read thread join") + .expect("read thread result"); + } + + #[test] + fn client_read_loop_disconnects_oversized_non_paste_input() { + let (mut client_stream, server_stream, _path) = + local_stream_pair("client-read-oversized-non-paste"); + let (server_event_tx, mut server_event_rx) = mpsc::channel(4); + let should_quit = Arc::new(AtomicBool::new(false)); + let read_quit = should_quit.clone(); + let handle = std::thread::spawn(move || { + client_read_loop(server_stream, 7, &server_event_tx, &read_quit) + }); + + protocol::write_message( + &mut client_stream, + &ClientMessage::Input { + data: vec![b'x'; MAX_INPUT_PAYLOAD + 1], + }, + ) + .expect("write oversized non-paste input"); + + assert!(matches!( + recv_server_event(&mut server_event_rx, "oversized non-paste disconnect"), + ServerEvent::ClientDisconnected { client_id: 7 } + )); + + drop(client_stream); + should_quit.store(true, Ordering::Release); + handle + .join() + .expect("read thread join") + .expect("read thread result"); + } + + #[test] + fn client_read_loop_disconnects_marker_wrapped_invalid_utf8() { + let (mut client_stream, server_stream, _path) = + local_stream_pair("client-read-invalid-utf8-paste"); + let (server_event_tx, mut server_event_rx) = mpsc::channel(4); + let should_quit = Arc::new(AtomicBool::new(false)); + let read_quit = should_quit.clone(); + let handle = std::thread::spawn(move || { + client_read_loop(server_stream, 7, &server_event_tx, &read_quit) + }); + let mut data = bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD + 1); + data[b"\x1b[200~".len()] = 0xff; + + protocol::write_message(&mut client_stream, &ClientMessage::Input { data }) + .expect("write marker-wrapped invalid UTF-8 input"); + + assert!(matches!( + recv_server_event(&mut server_event_rx, "invalid UTF-8 input disconnect"), + ServerEvent::ClientDisconnected { client_id: 7 } + )); + drop(client_stream); should_quit.store(true, Ordering::Release); handle @@ -1302,22 +1468,77 @@ new_tab = "ctrl+notakey" client_read_loop(server_stream, 7, &server_event_tx, &read_quit) }); + let maximum = vec![ + ClientInputEvent::Paste { + text: "x".repeat(MAX_INPUT_PAYLOAD / 2), + }, + ClientInputEvent::Paste { + text: "y".repeat(MAX_INPUT_PAYLOAD - (MAX_INPUT_PAYLOAD / 2)), + }, + ]; protocol::write_message( &mut client_stream, &ClientMessage::InputEvents { - events: vec![ClientInputEvent::Paste { - text: "x".repeat(MAX_INPUT_PAYLOAD + 1), - }], + events: maximum.clone(), }, ) - .expect("write oversized paste event"); + .expect("write maximum-size structured paste"); - match server_event_rx - .blocking_recv() - .expect("client disconnected event") - { - ServerEvent::ClientDisconnected { client_id } => assert_eq!(client_id, 7), - other => panic!("expected ClientDisconnected, got {other:?}"), + match recv_server_event(&mut server_event_rx, "maximum-size structured paste") { + ServerEvent::ClientInputEvents { client_id, events } => { + assert_eq!(client_id, 7); + assert_eq!(events, maximum); + } + other => panic!("expected maximum-size ClientInputEvents, got {other:?}"), + } + + let oversized = vec![ + ClientInputEvent::FocusGained, + ClientInputEvent::Paste { + text: "x".repeat(MAX_INPUT_PAYLOAD / 2), + }, + ClientInputEvent::Paste { + text: "y".repeat(MAX_INPUT_PAYLOAD - (MAX_INPUT_PAYLOAD / 2) + 1), + }, + ClientInputEvent::FocusLost, + ClientInputEvent::Paste { + text: "tail".to_owned(), + }, + ]; + protocol::write_message( + &mut client_stream, + &ClientMessage::InputEvents { events: oversized }, + ) + .expect("write oversized structured paste"); + + match recv_server_event(&mut server_event_rx, "oversized structured paste rejection") { + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + assert_eq!(client_id, 7); + assert_eq!(size, MAX_INPUT_PAYLOAD + 5); + assert_eq!(max, MAX_INPUT_PAYLOAD); + } + other => panic!("expected ClientPasteRejected, got {other:?}"), + } + + let valid = vec![ClientInputEvent::FocusGained]; + protocol::write_message( + &mut client_stream, + &ClientMessage::InputEvents { + events: valid.clone(), + }, + ) + .expect("write valid structured input after rejection"); + + match recv_server_event(&mut server_event_rx, "structured input after rejection") { + ServerEvent::ClientInputEvents { client_id, events } => { + assert_eq!(client_id, 7); + assert_eq!(events, valid); + } + other => panic!("expected ClientInputEvents after rejection, got {other:?}"), } drop(client_stream); diff --git a/src/server/headless.rs b/src/server/headless.rs index a2b8811a..a1c0e257 100644 --- a/src/server/headless.rs +++ b/src/server/headless.rs @@ -2797,6 +2797,23 @@ impl HeadlessServer { .collect(); self.handle_client_input_events(client_id, events) } + ServerEvent::ClientPasteRejected { + client_id, + size, + max, + } => { + self.send_to_client( + client_id, + ServerMessage::Notify { + kind: protocol::NotifyKind::Toast, + message: "Paste rejected".to_owned(), + body: Some(format!( + "Input message is {size} bytes; Herdr's limit is {max} bytes" + )), + }, + ); + false + } ServerEvent::ClientClipboardImage { client_id, extension, @@ -8694,6 +8711,78 @@ next_tab = "" ); } + #[test] + fn oversized_paste_rejection_notifies_only_the_sending_client() { + let mut server = test_headless_server(); + let (sender_writer, sender_control_rx, _sender_render_rx) = test_client_writer(); + let (foreground_writer, foreground_control_rx, _foreground_render_rx) = + test_client_writer(); + + server.clients.insert( + 1, + ClientConnection::new( + (120, 40), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 1, + RenderEncoding::SemanticFrame, + Some(sender_writer), + ), + ); + server.clients.insert( + 2, + ClientConnection::new( + (80, 24), + crate::kitty_graphics::HostCellSize::default(), + crate::terminal_theme::TerminalTheme::default(), + None, + 2, + RenderEncoding::SemanticFrame, + Some(foreground_writer), + ), + ); + server.foreground_client_id = Some(2); + server.sync_foreground_client_state(); + + assert!( + !server.handle_server_event(ServerEvent::ClientPasteRejected { + client_id: 1, + size: 5_000_012, + max: 1_048_576, + }) + ); + + match read_server_message( + sender_control_rx + .recv_timeout(Duration::from_millis(100)) + .expect("sending client rejection notification"), + ) { + ServerMessage::Notify { + kind, + message, + body, + } => { + assert_eq!(kind, protocol::NotifyKind::Toast); + assert_eq!(message, "Paste rejected"); + assert_eq!( + body.as_deref(), + Some("Input message is 5000012 bytes; Herdr's limit is 1048576 bytes") + ); + } + other => panic!("expected paste rejection notification, got {other:?}"), + } + assert!( + foreground_control_rx + .recv_timeout(Duration::from_millis(50)) + .is_err(), + "foreground client must not receive another client's rejection" + ); + assert_eq!(server.foreground_client_id, Some(2)); + assert_eq!(server.clients.len(), 2); + assert!(server.app.state.toast.is_none()); + } + #[test] fn herdr_toast_delivery_keeps_toast_in_frame_without_client_notify() { let mut server = test_headless_server(); -- cgit v1.2.3