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) <noreply@anthropic.com>
This commit is contained in:
@@ -403,7 +403,13 @@ fn build_engine(limits: Limits, logs: Option<Arc<Mutex<Vec<LogEntry>>>>) -> Rhai
|
|||||||
// Rhai's built-in `print` and `debug` map to stdout/stderr by
|
// Rhai's built-in `print` and `debug` map to stdout/stderr by
|
||||||
// default; we never want scripts dumping there directly. Disable
|
// default; we never want scripts dumping there directly. Disable
|
||||||
// them so scripts route all output through `log::*` instead.
|
// 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("print");
|
||||||
|
engine.disable_symbol("debug");
|
||||||
|
|
||||||
if let Some(logs) = logs {
|
if let Some(logs) = logs {
|
||||||
engine.register_static_module("log", build_log_module(logs).into());
|
engine.register_static_module("log", build_log_module(logs).into());
|
||||||
|
|||||||
@@ -49,6 +49,22 @@ fn validate_rejects_syntax_errors() {
|
|||||||
assert!(matches!(err, ExecError::Parse(_)));
|
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]
|
#[test]
|
||||||
fn returns_unwrapped_value_as_200_body() {
|
fn returns_unwrapped_value_as_200_body() {
|
||||||
let resp = engine()
|
let resp = engine()
|
||||||
|
|||||||
@@ -270,6 +270,16 @@ pub const SAFE_RENDER_FALLBACK: &str = "application/octet-stream";
|
|||||||
/// (`; charset=…`) are preserved when the base type is on the allowlist.
|
/// (`; charset=…`) are preserved when the base type is on the allowlist.
|
||||||
#[must_use]
|
#[must_use]
|
||||||
pub fn sanitize_stored_content_type(ct: &str) -> String {
|
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
|
let base = ct
|
||||||
.split(';')
|
.split(';')
|
||||||
.next()
|
.next()
|
||||||
@@ -446,4 +456,24 @@ mod content_type_sanitizer_tests {
|
|||||||
let ct = "text/plain; charset=utf-8";
|
let ct = "text/plain; charset=utf-8";
|
||||||
assert_eq!(sanitize_stored_content_type(ct), ct);
|
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"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user