From 6a4a19e24964e435badd0f5550028db8dfd29151 Mon Sep 17 00:00:00 2001 From: liutong Date: Fri, 21 Aug 2026 17:04:02 +0000 Subject: [PATCH] fix(security): sanitize response headers and cookies (CWE-113) Strip CR/LF from header values, validate header names against the HTTP token charset (RFC 7230), and sanitize cookie fields across all three backends (native, mongoose, JS). Native backend now passes cookies via the upstream cookies= parameter instead of manually joining with CRLF, eliminating the trusted response splitting pattern. Sanitization functions live in internal/header to avoid polluting the public API while sharing code between root and mongoose packages. Co-Authored-By: Claude Opus 4.6 --- internal/header/header.mbt | 49 ++++++++++++++++ internal/header/header_test.mbt | 90 ++++++++++++++++++++++++++++++ internal/header/moon.pkg | 7 +++ internal/header/pkg.generated.mbti | 15 +++++ mocket.js.mbt | 12 +++- mocket.native.mbt | 47 +++++++++++++--- moon.pkg | 1 + native/mongoose/mongoose.mbt | 13 ++++- native/mongoose/moon.pkg | 1 + 9 files changed, 222 insertions(+), 13 deletions(-) create mode 100644 internal/header/header.mbt create mode 100644 internal/header/header_test.mbt create mode 100644 internal/header/moon.pkg create mode 100644 internal/header/pkg.generated.mbti diff --git a/internal/header/header.mbt b/internal/header/header.mbt new file mode 100644 index 0000000..ecf7627 --- /dev/null +++ b/internal/header/header.mbt @@ -0,0 +1,49 @@ +///| +fn is_tchar(c : Char) -> Bool { + (c >= 'a' && c <= 'z') || + (c >= 'A' && c <= 'Z') || + (c >= '0' && c <= '9') || + c == '!' || + c == '#' || + c == '$' || + c == '%' || + c == '&' || + c == '\'' || + c == '*' || + c == '+' || + c == '-' || + c == '.' || + c == '^' || + c == '_' || + c == '`' || + c == '|' || + c == '~' +} + +///| +pub fn is_valid_header_name(name : String) -> Bool { + if name.length() == 0 { + return false + } + for c in name { + if !is_tchar(c) { + return false + } + } + true +} + +///| +pub fn sanitize_header_value(s : String) -> String { + if !s.contains("\r") && !s.contains("\n") { + return s + } + let buf = StringBuilder::new() + for c in s { + match c { + '\r' | '\n' => () + _ => buf.write_char(c) + } + } + buf.to_string() +} diff --git a/internal/header/header_test.mbt b/internal/header/header_test.mbt new file mode 100644 index 0000000..8563207 --- /dev/null +++ b/internal/header/header_test.mbt @@ -0,0 +1,90 @@ +///| +test "sanitize_header_value/clean" { + assert_eq(@header.sanitize_header_value("hello world"), "hello world") +} + +///| +test "sanitize_header_value/cr" { + assert_eq(@header.sanitize_header_value("foo\rbar"), "foobar") +} + +///| +test "sanitize_header_value/lf" { + assert_eq(@header.sanitize_header_value("foo\nbar"), "foobar") +} + +///| +test "sanitize_header_value/crlf" { + assert_eq(@header.sanitize_header_value("foo\r\nbar"), "foobar") +} + +///| +test "sanitize_header_value/multiple" { + assert_eq(@header.sanitize_header_value("a\rb\nc\r\nd"), "abcd") +} + +///| +test "sanitize_header_value/injection_attempt" { + assert_eq( + @header.sanitize_header_value("ok\r\nX-Injected: 1"), + "okX-Injected: 1", + ) +} + +///| +test "sanitize_header_value/empty" { + assert_eq(@header.sanitize_header_value(""), "") +} + +///| +test "is_valid_header_name/valid" { + assert_true(@header.is_valid_header_name("Content-Type")) + assert_true(@header.is_valid_header_name("X-Custom-Header")) + assert_true(@header.is_valid_header_name("Accept")) + assert_true(@header.is_valid_header_name("x-lowercase")) +} + +///| +test "is_valid_header_name/valid_special_chars" { + assert_true(@header.is_valid_header_name("!#$%&'*+-.^_`|~")) +} + +///| +test "is_valid_header_name/empty" { + assert_false(@header.is_valid_header_name("")) +} + +///| +test "is_valid_header_name/cr_lf" { + assert_false(@header.is_valid_header_name("X-Bad\r\nInjected")) + assert_false(@header.is_valid_header_name("X-Bad\rName")) + assert_false(@header.is_valid_header_name("X-Bad\nName")) +} + +///| +test "is_valid_header_name/space" { + assert_false(@header.is_valid_header_name("Bad Name")) +} + +///| +test "is_valid_header_name/colon" { + assert_false(@header.is_valid_header_name("Bad:Name")) +} + +///| +test "is_valid_header_name/parenthesis" { + assert_false(@header.is_valid_header_name("Bad(Name)")) +} + +///| +test "is_valid_header_name/at_sign" { + assert_false(@header.is_valid_header_name("Bad@Name")) +} + +///| +test "sanitize_header_value/cookie_injection" { + assert_eq( + @header.sanitize_header_value("session=abc\r\nSet-Cookie: evil=1"), + "session=abcSet-Cookie: evil=1", + ) +} diff --git a/internal/header/moon.pkg b/internal/header/moon.pkg new file mode 100644 index 0000000..6c28244 --- /dev/null +++ b/internal/header/moon.pkg @@ -0,0 +1,7 @@ +import { + "moonbitlang/core/test", +} + +warnings = "-15-29" + +supported_targets = "+js+native" diff --git a/internal/header/pkg.generated.mbti b/internal/header/pkg.generated.mbti new file mode 100644 index 0000000..a224ac6 --- /dev/null +++ b/internal/header/pkg.generated.mbti @@ -0,0 +1,15 @@ +// Generated using `moon info`, DON'T EDIT IT +package "oboard/mocket/internal/header" + +// Values +pub fn is_valid_header_name(String) -> Bool + +pub fn sanitize_header_value(String) -> String + +// Errors + +// Types and methods + +// Type aliases + +// Traits diff --git a/mocket.js.mbt b/mocket.js.mbt index 3f84508..5103b98 100644 --- a/mocket.js.mbt +++ b/mocket.js.mbt @@ -368,16 +368,24 @@ pub fn listen_ffi(mocket : Mocket, address : String) -> Unit { _ => HttpResponse::new(InternalServerError).body("Internal Server Error") } + let safe_headers : Map[@http.CaseInsensitiveString, String] = Map([]) + response.headers.each(fn(k, v) { + if @header.is_valid_header_name(Show::to_string(k)) { + safe_headers[k] = @header.sanitize_header_value(v.to_owned()) + } + }) res.write_head( response.status_code.to_int(), { - let mut headers_obj = @js.Value::from_json(response.headers.to_json()) catch { + let mut headers_obj = @js.Value::from_json(safe_headers.to_json()) catch { _ => @js.Object::new().to_value() } if !response.cookies.is_empty() { let cookies = response.cookies .values() - .map(fn(cookie) { Show::to_string(cookie) }) + .map(fn(cookie) { + @header.sanitize_header_value(Show::to_string(cookie)) + }) .to_array() set_js_property(headers_obj, "Set-Cookie", array_to_js(cookies)) } diff --git a/mocket.native.mbt b/mocket.native.mbt index 0721ed9..8860f46 100644 --- a/mocket.native.mbt +++ b/mocket.native.mbt @@ -338,21 +338,50 @@ async fn send_native_response( conn : @http.ServerConnection, response : HttpResponse, ) -> Unit { - let headers = view_headers_to_strings(response.headers) - if !response.cookies.is_empty() { - let cookies = response.cookies - .values() - .map(cookie => Show::to_string(cookie)) - .to_array() - headers.set("Set-Cookie", cookies.join("\r\nSet-Cookie: ")) - } - conn.send_response(response.status_code.to_int(), "OK", extra_headers=headers) + let raw_headers = view_headers_to_strings(response.headers) + let headers : Map[@http.CaseInsensitiveString, String] = Map([]) + raw_headers.each(fn(key, value) { + if @header.is_valid_header_name(Show::to_string(key)) { + headers[key] = @header.sanitize_header_value(value) + } + }) + let cookies = response.cookies + .values() + .map(cookie_item_to_http_cookie) + .to_array() + conn.send_response( + response.status_code.to_int(), + "OK", + extra_headers=headers, + cookies~, + ) if request.meth != Head && !response.raw_body.is_empty() { conn.write(response.raw_body) } conn.end_response() } +///| +fn cookie_item_to_http_cookie(item : CookieItem) -> @http.Cookie { + let extensions : Array[String] = [] + match item.same_site { + Some(Lax) => extensions.push("SameSite=Lax") + Some(Strict) => extensions.push("SameSite=Strict") + Some(SameSiteNone) => extensions.push("SameSite=None") + None => () + } + @http.Cookie( + @header.sanitize_header_value(item.name), + @header.sanitize_header_value(item.value), + path?=item.path.map(@header.sanitize_header_value), + domain?=item.domain.map(@header.sanitize_header_value), + max_age?=item.max_age.map(fn(a) { a.to_int64() }), + secure=item.secure.unwrap_or(false), + http_only=item.http_only.unwrap_or(false), + extensions~, + ) +} + ///| ///| diff --git a/moon.pkg b/moon.pkg index 40caff5..4fc602a 100644 --- a/moon.pkg +++ b/moon.pkg @@ -1,6 +1,7 @@ import { "oboard/mocket/js", "oboard/mocket/uri", + "oboard/mocket/internal/header", "moonbitlang/x/fs", "moonbitlang/x/crypto", "moonbitlang/x/path/posix", diff --git a/native/mongoose/mongoose.mbt b/native/mongoose/mongoose.mbt index e939d72..628d9b9 100644 --- a/native/mongoose/mongoose.mbt +++ b/native/mongoose/mongoose.mbt @@ -198,12 +198,21 @@ fn handle_request( } res.status(response.status_code.to_int()) response.headers.each((key, value) => { - res.set_header(to_cbytes(key), to_cbytes(value)) + let key_str = Show::to_string(key) + if @header.is_valid_header_name(key_str) { + res.set_header( + to_cbytes(key), + to_cbytes(@header.sanitize_header_value(value.to_owned())), + ) + } }) response.cookies .values() .each(cookie => { - res.set_header(to_cbytes("Set-Cookie"), to_cbytes(cookie.to_string())) + res.set_header( + to_cbytes("Set-Cookie"), + to_cbytes(@header.sanitize_header_value(Show::to_string(cookie))), + ) }) res.end_bytes(response.raw_body, response.raw_body.length()) }) diff --git a/native/mongoose/moon.pkg b/native/mongoose/moon.pkg index 840f198..c55afee 100644 --- a/native/mongoose/moon.pkg +++ b/native/mongoose/moon.pkg @@ -1,5 +1,6 @@ import { "oboard/mocket", + "oboard/mocket/internal/header", "moonbitlang/async/http", "moonbitlang/core/encoding/utf8", "moonbitlang/core/string",