From 95fddf31bcd67c7143597b28ac3e5eaf9d4579dc Mon Sep 17 00:00:00 2001 From: MechaCat02 Date: Fri, 12 Jun 2026 18:14:36 +0200 Subject: [PATCH] fix(audit-2026-06-11/C-2+F-SE-H-03): reject control chars in content-type; disable Rhai debug C-2 follow-up: sanitize_stored_content_type previously let a CRLF pass through an allowlisted prefix (e.g. "image/png\r\nX-Injected: 1" matched the image/ branch and returned the original), which would inject a response header / panic HeaderValue on download. Now any control byte (<0x20 or 0x7f) coerces to application/octet-stream up front. F-SE-H-03: disable_symbol("debug") to match "print" (the comment already claimed both were disabled) so scripts can't write attacker-controlled bytes to the operator's stderr. Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/executor-core/src/engine.rs | 6 ++++++ crates/executor-core/tests/engine.rs | 16 +++++++++++++++ crates/shared/src/files.rs | 30 ++++++++++++++++++++++++++++ 3 files changed, 52 insertions(+) diff --git a/crates/executor-core/src/engine.rs b/crates/executor-core/src/engine.rs index f8fec14..1790248 100644 --- a/crates/executor-core/src/engine.rs +++ b/crates/executor-core/src/engine.rs @@ -403,7 +403,13 @@ fn build_engine(limits: Limits, logs: Option>>>) -> Rhai // Rhai's built-in `print` and `debug` map to stdout/stderr by // default; we never want scripts dumping there directly. Disable // them so scripts route all output through `log::*` instead. + // + // Audit 2026-06-11 (F-SE-H-03) — `debug` was previously left + // enabled despite this comment, so a script could write + // attacker-controlled bytes (control chars, ANSI escapes) into the + // operator's stderr/journald stream. Disable it to match `print`. engine.disable_symbol("print"); + engine.disable_symbol("debug"); if let Some(logs) = logs { engine.register_static_module("log", build_log_module(logs).into()); diff --git a/crates/executor-core/tests/engine.rs b/crates/executor-core/tests/engine.rs index 7e29c9a..6f27b62 100644 --- a/crates/executor-core/tests/engine.rs +++ b/crates/executor-core/tests/engine.rs @@ -49,6 +49,22 @@ fn validate_rejects_syntax_errors() { assert!(matches!(err, ExecError::Parse(_))); } +#[test] +fn debug_and_print_symbols_are_disabled() { + // Audit 2026-06-11 (F-SE-H-03) — neither `print` nor `debug` may + // reach the host's stdout/stderr; both are disabled at the symbol + // level, so a script that calls them fails to parse/validate. + for src in [r#"print("x")"#, r#"debug("x")"#] { + let err = engine() + .validate(src) + .expect_err("disabled output symbol should be rejected"); + assert!( + matches!(err, ExecError::Parse(_)), + "{src} should be a parse-time rejection, got {err:?}" + ); + } +} + #[test] fn returns_unwrapped_value_as_200_body() { let resp = engine() diff --git a/crates/shared/src/files.rs b/crates/shared/src/files.rs index 4c562a8..9ed4c30 100644 --- a/crates/shared/src/files.rs +++ b/crates/shared/src/files.rs @@ -270,6 +270,16 @@ pub const SAFE_RENDER_FALLBACK: &str = "application/octet-stream"; /// (`; charset=…`) are preserved when the base type is on the allowlist. #[must_use] pub fn sanitize_stored_content_type(ct: &str) -> String { + // Audit 2026-06-11 follow-up — reject any control character first. + // The download handler plumbs the (sanitized) content_type straight + // into a response header; an embedded CR/LF would otherwise survive + // the allowlist (e.g. "image/png\r\nX-Injected: 1" matches the + // `image/` prefix) and then panic `HeaderValue` construction on + // serve. Coercing to the opaque fallback closes the response-header + // injection / handler-panic vector for the stored value. + if ct.bytes().any(|b| b < 0x20 || b == 0x7f) { + return SAFE_RENDER_FALLBACK.to_string(); + } let base = ct .split(';') .next() @@ -446,4 +456,24 @@ mod content_type_sanitizer_tests { let ct = "text/plain; charset=utf-8"; assert_eq!(sanitize_stored_content_type(ct), ct); } + + #[test] + fn control_chars_are_coerced_even_under_an_allowlisted_prefix() { + // Audit 2026-06-11 follow-up — a CRLF in an otherwise + // allowlisted type must NOT pass through (it would inject a + // response header / panic HeaderValue on download). + for ct in [ + "image/png\r\nX-Injected: 1", + "image/png\nX-Injected: 1", + "text/plain\r\nSet-Cookie: x=1", + "audio/mpeg\u{7f}", + "video/mp4\u{0}", + ] { + assert_eq!( + sanitize_stored_content_type(ct), + SAFE_RENDER_FALLBACK, + "{ct:?} must coerce" + ); + } + } }