fix: reject oversized text pastes safely (#1676)
refs #1665 Co-authored-by: akbash-bot <300245827+akbash-bot@users.noreply.github.com>
This commit is contained in:
parent
fd29addd71
commit
6382bd433a
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
### Fixed
|
||||
- Linux sound notifications now terminate and reap audio players that do not exit, preventing unavailable audio from leaving CPU-bound `mpg123` processes behind. (#1622)
|
||||
- Oversized bracketed text pastes are now rejected with a client-local notification instead of disconnecting the client. (#1665)
|
||||
- Agent prompt waits now report `agent_prompt_stalled` after five seconds without an observed state change instead of waiting indefinitely after an ineffective submission.
|
||||
- `herdr config check` now reports unknown config keys with their full paths instead of treating ignored typos as valid configuration. (#1573)
|
||||
- Codex panes with customized static terminal titles now fall back to the live working footer instead of remaining idle, while OSC activity remains preferred. (#1563)
|
||||
|
|
|
|||
|
|
@ -107,6 +107,21 @@ pub(crate) const GHOSTTY_COLOR_SCHEME_LIGHT_REPORT: &[u8] = b"\x1b[?997;2n";
|
|||
const BRACKETED_PASTE_START: &[u8] = b"\x1b[200~";
|
||||
const BRACKETED_PASTE_END: &[u8] = b"\x1b[201~";
|
||||
|
||||
/// Returns whether `data` is exactly one complete bracketed-paste sequence.
|
||||
///
|
||||
/// Client transport uses this to distinguish recoverable oversized interactive
|
||||
/// pastes from generic oversized input, which remains a protocol violation.
|
||||
pub(crate) fn is_complete_text_bracketed_paste(data: &[u8]) -> bool {
|
||||
if !data.starts_with(BRACKETED_PASTE_START) {
|
||||
return false;
|
||||
}
|
||||
let Some(end) = find_subsequence(data, BRACKETED_PASTE_END) else {
|
||||
return false;
|
||||
};
|
||||
end + BRACKETED_PASTE_END.len() == data.len()
|
||||
&& std::str::from_utf8(&data[BRACKETED_PASTE_START.len()..end]).is_ok()
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
pub enum RawInputEvent {
|
||||
Key(TerminalKey),
|
||||
|
|
@ -1134,6 +1149,19 @@ mod tests {
|
|||
assert_eq!(consumed, 17);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn complete_text_bracketed_paste_requires_one_exact_utf8_sequence() {
|
||||
assert!(is_complete_text_bracketed_paste(b"\x1b[200~hello\x1b[201~"));
|
||||
assert!(!is_complete_text_bracketed_paste(b"\x1b[200~hello"));
|
||||
assert!(!is_complete_text_bracketed_paste(
|
||||
b"\x1b[200~hello\x1b[201~rest"
|
||||
));
|
||||
assert!(!is_complete_text_bracketed_paste(
|
||||
b"\x1b[200~one\x1b[201~\x1b[200~two\x1b[201~"
|
||||
));
|
||||
assert!(!is_complete_text_bracketed_paste(b"\x1b[200~\xff\x1b[201~"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn parses_sgr_mouse() {
|
||||
let (RawInputEvent::Mouse(mouse), consumed) = extract_one_event(b"\x1b[<0;20;10M").unwrap()
|
||||
|
|
|
|||
|
|
@ -300,6 +300,12 @@ pub(crate) enum ServerEvent {
|
|||
client_id: u64,
|
||||
events: Vec<crate::protocol::ClientInputEvent>,
|
||||
},
|
||||
/// 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<ServerEvent>, 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<u8> {
|
||||
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<ClientWriterQueue>) {
|
||||
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));
|
||||
|
|
@ -1184,21 +1249,122 @@ new_tab = "ctrl+notakey"
|
|||
client_read_loop(server_stream, 7, &server_event_tx, &read_quit)
|
||||
});
|
||||
|
||||
protocol::write_message(
|
||||
&mut client_stream,
|
||||
&ClientMessage::Input {
|
||||
data: bracketed_paste_with_total_len(MAX_INPUT_PAYLOAD),
|
||||
},
|
||||
)
|
||||
.expect("write maximum-size bracketed paste");
|
||||
|
||||
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 input");
|
||||
.expect("write oversized non-paste input");
|
||||
|
||||
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:?}"),
|
||||
}
|
||||
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);
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
|
|
@ -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();
|
||||
|
|
|
|||
Loading…
Reference in New Issue