From d2939de936d0d2c50f03a719a8ea01d837f9b4bc Mon Sep 17 00:00:00 2001 From: zach Date: Sat, 11 Apr 2026 00:39:21 +0200 Subject: [PATCH] fix escape code leak --- parse_test.rs | 2 ++ src/lib.rs | 1 + src/vt_parser.rs | 82 +++++++++++++++++++++++++++++++++++++--------- src/vt_test_osc.rs | 78 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 147 insertions(+), 16 deletions(-) create mode 100644 parse_test.rs create mode 100644 src/vt_test_osc.rs diff --git a/parse_test.rs b/parse_test.rs new file mode 100644 index 0000000..66f12d9 --- /dev/null +++ b/parse_test.rs @@ -0,0 +1,2 @@ +use zterm::vt_parser::*; +fn main() {} diff --git a/src/lib.rs b/src/lib.rs index 7ba9b50..1f5246b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -20,3 +20,4 @@ pub mod statusline; pub mod terminal; pub mod simd_utf8; pub mod vt_parser; +mod vt_test_osc; diff --git a/src/vt_parser.rs b/src/vt_parser.rs index 6cd03fc..c6223ec 100644 --- a/src/vt_parser.rs +++ b/src/vt_parser.rs @@ -482,7 +482,9 @@ impl SharedParser { // Like Kitty line 1516: consume_input(self, ...) let made_progress = self.consume_input(handler); - parsed_any = true; + if made_progress { + parsed_any = true; + } // Re-acquire lock state = self.state.lock().unwrap(); @@ -635,14 +637,14 @@ impl SharedParser { true } State::Csi => { - // Like Kitty lines 1465-1466: - // if (consume_csi(self)) { self->read.consumed = self->read.pos; if (self->csi.is_valid) dispatch_csi(self); SET_STATE(NORMAL); } + let state_before = *vte_state; if Self::consume_csi_impl( handler, buf, parse_pos, parse_sz, - *parse_consumed, + parse_consumed, + vte_state, csi, escape_len, ) { @@ -650,30 +652,41 @@ impl SharedParser { if csi.is_valid { handler.csi(csi); } - *vte_state = State::Normal; + if *vte_state == state_before { + *vte_state = State::Normal; + } true } else { - false + *vte_state != state_before } } State::Osc => { + let state_before = *vte_state; if Self::consume_osc_impl( - handler, buf, parse_pos, parse_sz, vte_state, osc_buffer, + handler, + buf, + parse_pos, + parse_sz, + parse_consumed, + vte_state, + osc_buffer, escape_len, ) { *parse_consumed = *parse_pos; *vte_state = State::Normal; true } else { - false + *vte_state != state_before } } State::Dcs | State::Apc | State::Pm | State::Sos => { + let state_before = *vte_state; if Self::consume_string_impl( handler, buf, parse_pos, parse_sz, + parse_consumed, vte_state, string_buffer, escape_len, @@ -682,7 +695,7 @@ impl SharedParser { *vte_state = State::Normal; true } else { - false + *vte_state != state_before } } }; @@ -837,6 +850,11 @@ impl SharedParser { b'\\' => { *vte_state = State::Normal; } + 0x1B => { + // ESC followed by ESC. Start new escape sequence. + *vte_state = State::Escape; + return true; + } _ => { log::debug!("Unknown escape sequence: ESC {:02x}", ch); *vte_state = State::Normal; @@ -913,7 +931,8 @@ impl SharedParser { buf: &[u8; BUF_SIZE], parse_pos: &mut usize, parse_sz: usize, - parse_consumed: usize, + parse_consumed: &mut usize, + vte_state: &mut State, csi: &mut CsiParams, escape_len: &mut usize, ) -> bool { @@ -922,8 +941,15 @@ impl SharedParser { *parse_pos += 1; *escape_len += 1; + if ch == 0x1B { + *vte_state = State::Escape; + *parse_consumed = *parse_pos; + *escape_len = 0; + return false; // Aborted by new ESC + } + // Handle embedded control characters - if ch <= 0x1F && ch != 0x1B { + if ch <= 0x1F { handler.control(ch); continue; } @@ -1023,7 +1049,7 @@ impl SharedParser { } // Check max length - if *parse_pos - parse_consumed > MAX_ESCAPE_LEN { + if *parse_pos - *parse_consumed > MAX_ESCAPE_LEN { log::debug!("CSI escape too long, ignoring"); return true; } @@ -1037,6 +1063,7 @@ impl SharedParser { buf: &[u8; BUF_SIZE], parse_pos: &mut usize, parse_sz: usize, + parse_consumed: &mut usize, vte_state: &mut State, osc_buffer: &mut Vec, escape_len: &mut usize, @@ -1069,6 +1096,7 @@ impl SharedParser { *parse_pos += 1; handler.osc(osc_buffer); *vte_state = State::Escape; + *parse_consumed = *parse_pos; *escape_len = 0; return false; } else { @@ -1098,6 +1126,7 @@ impl SharedParser { buf: &[u8; BUF_SIZE], parse_pos: &mut usize, parse_sz: usize, + parse_consumed: &mut usize, vte_state: &mut State, string_buffer: &mut Vec, escape_len: &mut usize, @@ -1128,10 +1157,17 @@ impl SharedParser { ); return true; } else if *parse_pos + 1 < parse_sz { - // ESC not followed by \ - include in buffer - string_buffer.push(ch); + // ESC not followed by \ - abort string, start new escape *parse_pos += 1; - *escape_len += 1; + Self::dispatch_string_command( + handler, + vte_state, + string_buffer, + ); + *vte_state = State::Escape; + *parse_consumed = *parse_pos; + *escape_len = 0; + return false; } else { // ESC at end of buffer - need more data return false; @@ -1368,6 +1404,11 @@ impl Parser { self.state = State::Normal; 1 } + 0x1B => { + // ESC followed by ESC. Start new escape sequence. + self.state = State::Escape; + 1 + } _ => { // Unknown escape sequence, ignore and return to normal log::debug!("Unknown escape sequence: ESC {:02x}", ch); @@ -1442,8 +1483,17 @@ impl Parser { return consumed; } + if ch == 0x1B { + self.state = State::Escape; + self.escape_len = 0; + // Wait! If it's ESC, we consumed it. But the next loop in parse() will call consume_escape, + // which EXPECTS the char AFTER ESC. + // So consuming it is PERFECT! + return consumed; + } + // Handle control characters embedded in CSI (common to all states) - if ch <= 0x1F && ch != 0x1B { + if ch <= 0x1F { handler.control(ch); continue; } diff --git a/src/vt_test_osc.rs b/src/vt_test_osc.rs new file mode 100644 index 0000000..6506f75 --- /dev/null +++ b/src/vt_test_osc.rs @@ -0,0 +1,78 @@ +use crate::vt_parser::*; + +pub struct DummyHandler { + pub text: String, + pub osc_calls: Vec>, +} +impl Handler for DummyHandler { + fn text(&mut self, codepoints: &[u32]) { + for &c in codepoints { + if let Some(ch) = char::from_u32(c) { + self.text.push(ch); + } + } + } + fn control(&mut self, _byte: u8) {} + fn csi(&mut self, _csi: &CsiParams) {} + fn osc(&mut self, osc: &[u8]) { + self.osc_calls.push(osc.to_vec()); + } + fn save_cursor(&mut self) {} + fn restore_cursor(&mut self) {} + fn reset(&mut self) {} + fn index(&mut self) {} + fn newline(&mut self) {} + fn reverse_index(&mut self) {} + fn screen_alignment(&mut self) {} + fn set_tab_stop(&mut self) {} + fn set_keypad_mode(&mut self, _mode: bool) {} + fn add_vt_parser_ns(&mut self, _ns: u64) {} +} + +#[test] +fn test_osc_leak_byte_by_byte() { + let parser = SharedParser::new(); + let mut handler = DummyHandler { text: String::new(), osc_calls: Vec::new() }; + + let data = b"\x1b]4;1;#769E00\x1b\\\x1b]4;2;#93DE88\x1b\\"; + + for &byte in data { + let (ptr, _) = parser.create_write_buffer(); + unsafe { + *ptr = byte; + } + parser.commit_write(1); + + while parser.run_parse_pass(&mut handler) {} + } + + println!("TEXT: {:?}", handler.text); + println!("OSC calls: {}", handler.osc_calls.len()); + for call in &handler.osc_calls { + println!(" OSC: {:?}", std::str::from_utf8(call).unwrap()); + } + + assert_eq!(handler.text, "", "Text should be empty, but leaked escape sequence bytes!"); + assert_eq!(handler.osc_calls.len(), 2, "Should have parsed exactly two OSC calls"); +} + +#[test] +fn test_csi_aborted_by_osc() { + let parser = SharedParser::new(); + let mut handler = DummyHandler { text: String::new(), osc_calls: Vec::new() }; + + // An incomplete CSI sequence aborted by an OSC sequence + let data = b"\x1b[38;2;255;0;0\x1b]4;1;#769E00\x1b\\"; + + let (ptr, len) = parser.create_write_buffer(); + assert!(len >= data.len()); + unsafe { + std::ptr::copy_nonoverlapping(data.as_ptr(), ptr, data.len()); + } + parser.commit_write(data.len()); + + while parser.run_parse_pass(&mut handler) {} + + assert_eq!(handler.text, "", "Text should be empty, but leaked escape sequence bytes!"); + assert_eq!(handler.osc_calls.len(), 1, "Should have parsed the OSC sequence even after aborting CSI"); +}