Harden cookie attribute validation

.changeset/eff-210-cookie-validation.md

@@ -0,0 +1,5 @@
+---
+"effect": patch
+---
+
+Validate cookie names, domains, and paths before constructing or serializing cookies.

packages/effect/src/unstable/http/Cookies.ts

@@ -426,6 +426,10 @@ export const isEmpty = (self: Cookies): boolean => Record.isEmptyRecord(self.coo
 
 // oxlint-disable-next-line no-control-regex
 const fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/
+const cookieNameRegExp = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/
+// oxlint-disable-next-line no-control-regex
+const cookieDomainRegExp = /^[\u0009\u0020-\u003a\u003c-\u007e\u0080-\u00ff]+$/
+const cookiePathRegExp = /^[\u0020-\u003a\u003c-\u007e]+$/
 
 const CookieProto = {
   [CookieTypeId]: CookieTypeId,
@@ -455,26 +459,10 @@ export function makeCookie(
   value: string,
   options?: Cookie["options"] | undefined
 ): Result.Result<Cookie, CookiesError> {
-  if (!fieldContentRegExp.test(name)) {
-    return Result.fail(CookiesError.fromReason("InvalidCookieName"))
-  }
   const encodedValue = encodeURIComponent(value)
-  if (encodedValue && !fieldContentRegExp.test(encodedValue)) {
-    return Result.fail(CookiesError.fromReason("InvalidCookieValue"))
-  }
-
-  if (options !== undefined) {
-    if (options.domain !== undefined && !fieldContentRegExp.test(options.domain)) {
-      return Result.fail(CookiesError.fromReason("InvalidCookieDomain"))
-    }
-
-    if (options.path !== undefined && !fieldContentRegExp.test(options.path)) {
-      return Result.fail(CookiesError.fromReason("InvalidCookiePath"))
-    }
-
-    if (options.maxAge !== undefined && !Duration.isFinite(Duration.fromInputUnsafe(options.maxAge))) {
-      return Result.fail(CookiesError.fromReason("CookieInfinityMaxAge"))
-    }
+  const error = validateCookie(name, encodedValue, options)
+  if (error !== undefined) {
+    return Result.fail(error)
   }
 
   return Result.succeed(Object.assign(Object.create(CookieProto), {
@@ -485,6 +473,28 @@ export function makeCookie(
   }))
 }
 
+function validateCookie(
+  name: string,
+  encodedValue: string,
+  options: Cookie["options"] | undefined
+): CookiesError | undefined {
+  if (!cookieNameRegExp.test(name)) {
+    return CookiesError.fromReason("InvalidCookieName")
+  }
+  if (encodedValue && !fieldContentRegExp.test(encodedValue)) {
+    return CookiesError.fromReason("InvalidCookieValue")
+  }
+  if (options?.domain !== undefined && !cookieDomainRegExp.test(options.domain)) {
+    return CookiesError.fromReason("InvalidCookieDomain")
+  }
+  if (options?.path !== undefined && !cookiePathRegExp.test(options.path)) {
+    return CookiesError.fromReason("InvalidCookiePath")
+  }
+  if (options?.maxAge !== undefined && !Duration.isFinite(Duration.fromInputUnsafe(options.maxAge))) {
+    return CookiesError.fromReason("CookieInfinityMaxAge")
+  }
+}
+
 /**
  * Create a new cookie, throwing an error if invalid
  *
@@ -779,6 +789,10 @@ export const setAllUnsafe: {
  * @since 4.0.0
  */
 export function serializeCookie(self: Cookie): string {
+  const error = validateCookie(self.name, self.valueEncoded, self.options)
+  if (error !== undefined) {
+    throw error
+  }
   let str = self.name + "=" + self.valueEncoded
 
   if (self.options === undefined) {

packages/effect/test/unstable/http/Cookies.test.ts

@@ -1,12 +1,66 @@
 import { assert, describe, it } from "@effect/vitest"
 import { assertNone, assertSome, deepStrictEqual } from "@effect/vitest/utils"
-import { Schema } from "effect"
+import { Result, Schema } from "effect"
 import * as Option from "effect/Option"
 import { TestSchema } from "effect/testing"
 import { Cookies } from "effect/unstable/http"
-import { assertSuccess } from "../../utils/assert.ts"
+import { assertFailure, assertSuccess } from "../../utils/assert.ts"
 
 describe("Cookies", () => {
+  describe("makeCookie", () => {
+    it("rejects cookie attribute delimiters in names, domains, and paths", () => {
+      assertFailure(
+        Cookies.makeCookie("a; Domain=evil.com; b", "token"),
+        Cookies.CookiesError.fromReason("InvalidCookieName")
+      )
+      assertFailure(
+        Cookies.makeCookie("session", "token", { domain: "legit.com; Domain=.parent.tld" }),
+        Cookies.CookiesError.fromReason("InvalidCookieDomain")
+      )
+      assertFailure(
+        Cookies.makeCookie("session", "token", { path: "/; HttpOnly" }),
+        Cookies.CookiesError.fromReason("InvalidCookiePath")
+      )
+    })
+
+    it("accepts RFC 6265 token names and legitimate domains and paths", () => {
+      for (const domain of ["sub.example.com", ".sub.example.com"]) {
+        const cookie = Result.getOrThrow(
+          Cookies.makeCookie("!#$%&'*+-.^_`|~", "token", {
+            domain,
+            path: "/some-path_with~chars/%20"
+          })
+        )
+
+        assert.strictEqual(
+          Cookies.serializeCookie(cookie),
+          `!#$%&'*+-.^_\`|~=token; Domain=${domain}; Path=/some-path_with~chars/%20`
+        )
+      }
+    })
+  })
+
+  describe("toSetCookieHeaders", () => {
+    const invalidCookie = {
+      name: "session",
+      value: "token",
+      valueEncoded: "token",
+      options: { domain: "legit.com; Domain=.evil.com" }
+    } as unknown as Cookies.Cookie
+
+    it("rejects invalid cookies supplied through fromIterable", () => {
+      const cookies = Cookies.fromIterable([invalidCookie])
+
+      assert.throws(() => Cookies.toSetCookieHeaders(cookies), /InvalidCookieDomain/)
+    })
+
+    it("rejects invalid cookies supplied through setCookie", () => {
+      const cookies = Cookies.setCookie(Cookies.empty, invalidCookie)
+
+      assert.throws(() => Cookies.toSetCookieHeaders(cookies), /InvalidCookieDomain/)
+    })
+  })
+
   it("expireCookie returns a Result with an expired Set-Cookie value", () => {
     assertSuccess(
       Cookies.expireCookie(Cookies.empty, "session", { path: "/", secure: true }),