fix: strip CRLF from response header values

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()
+}

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",
+  )
+}

internal/header/moon.pkg

@@ -0,0 +1,7 @@
+import {
+  "moonbitlang/core/test",
+}
+
+warnings = "-15-29"
+
+supported_targets = "+js+native"

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

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))
           }

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~,
+  )
+}
+
 ///|
 
 ///|

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",

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())
   })

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",