fix(cdp): truncate client-controlled log previews on char boundaries (#397)

server.rs logged previews of attacker-controlled strings with raw byte
slicing: `&text[..text.len().min(200)]` for a malformed CDP frame (inside a
`warn!`, which is active at the worker's default log level) and the same for
an intercepted event JSON. `&s[..n]` panics when byte `n` falls inside a
multi-byte UTF-8 character, so a single WebSocket frame that is invalid JSON,
>=201 bytes, with a multi-byte char straddling byte 200 panics the CDP
processor task — an unauthenticated one-frame crash. page.rs had the same
pattern (behind --verbose debug logging) for evaluated JS expressions.

Add truncate_on_char_boundary (str::floor_char_boundary is still unstable) and
route all four sites through it. Regression test constructs the exact 'a'*199 +
'€' input, asserts the old raw slice panics (via catch_unwind) and the helper
truncates on the boundary instead.

crates/obscura-browser/src/page.rs

@@ -67,6 +67,21 @@ fn hex_val(b: u8) -> Option<u8> {
     }
 }
 
+/// Truncate `s` to at most `max` bytes without splitting a UTF-8 character.
+/// `&s[..max]` panics if `max` lands inside a multi-byte char; the evaluated
+/// expression logged below is caller-controlled, so slice it safely.
+/// (`str::floor_char_boundary` would do this but is still unstable.)
+fn truncate_on_char_boundary(s: &str, max: usize) -> &str {
+    if s.len() <= max {
+        return s;
+    }
+    let mut end = max;
+    while end > 0 && !s.is_char_boundary(end) {
+        end -= 1;
+    }
+    &s[..end]
+}
+
 #[cfg(feature = "stealth")]
 use obscura_net::StealthHttpClient;
 
@@ -1273,7 +1288,7 @@ impl Page {
             match js.evaluate_with_timeout(expression, timeout) {
                 Ok(val) => val,
                 Err(e) => {
-                    tracing::debug!("JS eval error/timeout for '{}': {}", &expression[..expression.len().min(80)], e);
+                    tracing::debug!("JS eval error/timeout for '{}': {}", truncate_on_char_boundary(expression, 80), e);
                     serde_json::Value::Null
                 }
             }
@@ -1287,7 +1302,7 @@ impl Page {
             match js.evaluate(expression) {
                 Ok(val) => val,
                 Err(e) => {
-                    tracing::debug!("JS eval error for '{}': {}", &expression[..expression.len().min(80)], e);
+                    tracing::debug!("JS eval error for '{}': {}", truncate_on_char_boundary(expression, 80), e);
                     serde_json::Value::Null
                 }
             }
@@ -1685,7 +1700,19 @@ fn url_matches_cdp_pattern(pattern: &str, url: &str) -> bool {
 
 #[cfg(test)]
 mod tests {
-    use super::url_matches_cdp_pattern;
+    use super::{truncate_on_char_boundary, url_matches_cdp_pattern};
+
+    #[test]
+    fn truncate_never_splits_a_multibyte_char() {
+        // A caller-supplied expression whose byte 80 lands inside a multi-byte
+        // char would make `&expression[..80]` panic; the helper truncates safely.
+        let s = format!("{}€tail", "a".repeat(79));
+        assert!(!s.is_char_boundary(80), "setup: byte 80 splits the € char");
+        let t = truncate_on_char_boundary(&s, 80);
+        assert!(s.starts_with(t));
+        assert_eq!(t.len(), 79, "should stop right before the € char");
+        assert_eq!(truncate_on_char_boundary("short", 80), "short");
+    }
 
     #[test]
     fn url_matches_cdp_pattern_handles_wildcards_across_url_parts() {

crates/obscura-cdp/src/server.rs

@@ -717,7 +717,7 @@ async fn process_with_interception(
                     "sessionId": session_for_events,
                 });
                 let event_str = event_json.to_string();
-                tracing::info!("INTERCEPTION event JSON: {}", &event_str[..event_str.len().min(300)]);
+                tracing::info!("INTERCEPTION event JSON: {}", crate::util::truncate_on_char_boundary(&event_str, 300));
                 let _ = reply_tx.send(event_str);
                 intercepted_paused.insert(intercepted.request_id.clone(), intercepted.resolver);
                 tokio::time::sleep(tokio::time::Duration::from_millis(50)).await;
@@ -829,7 +829,7 @@ async fn process_cdp_message(
     let req: CdpRequest = match serde_json::from_str(text) {
         Ok(r) => r,
         Err(e) => {
-            warn!("Invalid CDP: {}: {}", e, &text[..text.len().min(200)]);
+            warn!("Invalid CDP: {}: {}", e, crate::util::truncate_on_char_boundary(text, 200));
             return;
         }
     };

crates/obscura-cdp/src/util.rs

@@ -18,6 +18,24 @@ pub(crate) fn url_is_file_scheme(raw: &str) -> bool {
         })
 }
 
+/// Truncate `s` to at most `max` bytes, never splitting a UTF-8 character.
+///
+/// `&s[..max]` panics when `max` lands inside a multi-byte character, and the
+/// strings we truncate for log previews are attacker-controlled (raw WebSocket
+/// frames, intercepted URLs). A single frame whose byte `max` straddles a
+/// multi-byte char would otherwise panic the CDP processor task
+/// (`str::floor_char_boundary` would do this but is still unstable).
+pub(crate) fn truncate_on_char_boundary(s: &str, max: usize) -> &str {
+    if s.len() <= max {
+        return s;
+    }
+    let mut end = max;
+    while end > 0 && !s.is_char_boundary(end) {
+        end -= 1;
+    }
+    &s[..end]
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -59,4 +77,34 @@ mod tests {
         assert!(!url_is_file_scheme("notfile:///x"));
         assert!(!url_is_file_scheme("http://file/"));
     }
+
+    #[test]
+    fn truncate_never_splits_a_multibyte_char() {
+        // 199 ASCII bytes + '€' (3 bytes, occupying indices 199..=201): byte 200
+        // falls inside the '€'. This is exactly the shape of a malformed CDP
+        // frame that would reach the `warn!("Invalid CDP: ...")` preview.
+        let s = format!("{}€tail", "a".repeat(199));
+        assert!(!s.is_char_boundary(200), "setup: byte 200 splits the € char");
+
+        // The old logging code did `&s[..s.len().min(200)]`, which panics here —
+        // a single crafted frame would take down the CDP processor task.
+        let naive = std::panic::catch_unwind(|| {
+            let _ = &s[..s.len().min(200)];
+        });
+        assert!(naive.is_err(), "raw byte slice at a non-char-boundary must panic");
+
+        // The helper truncates on the boundary before the € instead.
+        let safe = truncate_on_char_boundary(&s, 200);
+        assert!(s.starts_with(safe));
+        assert!(safe.len() <= 200);
+        assert_eq!(safe.len(), 199, "should stop right before the € char");
+    }
+
+    #[test]
+    fn truncate_returns_whole_string_when_short() {
+        assert_eq!(truncate_on_char_boundary("hi", 200), "hi");
+        assert_eq!(truncate_on_char_boundary("", 10), "");
+        // Exact-length and exact-boundary cases are returned unchanged.
+        assert_eq!(truncate_on_char_boundary("abc", 3), "abc");
+    }
 }