Skip to content

fix: strip CRLF from response header values - #14

Open
I3eg1nner wants to merge 1 commit into
oboard:mainfrom
I3eg1nner:fix/cwe113-crlf-header-injection
Open

fix: strip CRLF from response header values#14
I3eg1nner wants to merge 1 commit into
oboard:mainfrom
I3eg1nner:fix/cwe113-crlf-header-injection

Conversation

@I3eg1nner

@I3eg1nner I3eg1nner commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Summary

Fix HTTP response header injection (CWE-113) across all three backends.

Changes per review feedback:

  • [P0] Rebased onto latest main — adapted to the unified dispatch_http flow; no execute_middlewares references remain.
  • [P1] Header name validation — added is_valid_header_name() that validates against the RFC 7230 token charset. Headers with invalid names are silently dropped in all three backends.
  • [P1] Native cookie handling — replaced the cookies.join("\r\nSet-Cookie: ") CRLF-join hack with the upstream cookies= parameter on ServerConnection::send_response. Added cookie_item_to_http_cookie to convert CookieItem@http.Cookie, sanitizing name/value/path/domain fields.
  • [P2] Internal package — moved sanitize_header_value and is_valid_header_name to internal/header, shared by root and mongoose packages. Root pkg.generated.mbti no longer exposes implementation details.
  • [P2] Tests — 16 test cases covering CR/LF/CRLF stripping, header name validation (token charset, empty, space, colon, parenthesis, at-sign), and cookie injection patterns.

Verification

moon check --target js,native --deny-warn  # 0 errors
moon test --target js                       # 103 passed
moon test --target native                   # 98 passed
moon fmt && moon info --target native       # no diff

Co-Authored-By: Claude Opus 4.6 noreply@anthropic.com

@oboard

oboard commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Error: [4014]
╭─[ /home/runner/work/mocket/mocket/mocket.native.mbt:364:39 ]

364 │ raw_headers.each(fn(k, v) { headers[k] = sanitize_header_value(v) })
│ ┬
│ ╰── Expr Type Mismatch
has type : @moonbitlang/async/http.CaseInsensitiveString
wanted : String
─────╯
Error: [0020]
╭─[ /home/runner/work/mocket/mocket/mocket.native.mbt:368:44 ]

368 │ .map(fn(c) { sanitize_header_value(c.to_string()) })
│ ────┬────
│ ╰────── Error Warning (deprecated): The method to_string is implicitly promoted from impl Show for CookieItem. This behavior is deprecated, either use Show::to_string instead or add a extend CookieItem with Show::{to_string, ..} declaration.
─────╯
Error: [4014]
╭─[ /home/runner/work/mocket/mocket/mocket.native.mbt:372:73 ]

372 │ conn.send_response(response.status_code.to_int(), "OK", extra_headers=headers)
│ ───┬───
│ ╰───── Expr Type Mismatch
has type : Map[String, String]
wanted : Map[@moonbitlang/async/http.CaseInsensitiveString, String]
─────╯

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 <noreply@anthropic.com>
@I3eg1nner
I3eg1nner force-pushed the fix/cwe113-crlf-header-injection branch from d6a6c20 to 6a4a19e Compare August 21, 2026 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants