Harden v3 cookie attribute validation

.changeset/tidy-cookies-smile.md

@@ -0,0 +1,5 @@
+---
+"@effect/platform": patch
+---
+
+Reject invalid cookie names and cookie attribute separators in domains and paths.

packages/platform/src/Cookies.ts

@@ -167,7 +167,7 @@ function parseSetCookie(header: string): Option.Option<Cookie> {
     return Option.none()
   }
   const name = parts[0].slice(0, firstEqual)
-  if (!fieldContentRegExp.test(name)) {
+  if (!cookieNameRegExp.test(name)) {
     return Option.none()
   }
 
@@ -302,7 +302,11 @@ export const empty: Cookies = fromIterable([])
 export const isEmpty = (self: Cookies): boolean => Record.isEmptyRecord(self.cookies)
 
 // eslint-disable-next-line no-control-regex
-const fieldContentRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/
+const cookieValueRegExp = /^[\u0009\u0020-\u007e\u0080-\u00ff]+$/
+const cookieNameRegExp = /^[!#$%&'*+\-.^_`|~0-9A-Za-z]+$/
+// eslint-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,
@@ -328,20 +332,20 @@ export function makeCookie(
   value: string,
   options?: Cookie["options"] | undefined
 ): Either.Either<Cookie, CookiesError> {
-  if (!fieldContentRegExp.test(name)) {
+  if (!cookieNameRegExp.test(name)) {
     return Either.left(new CookiesError({ reason: "InvalidName" }))
   }
   const encodedValue = encodeURIComponent(value)
-  if (encodedValue && !fieldContentRegExp.test(encodedValue)) {
+  if (encodedValue && !cookieValueRegExp.test(encodedValue)) {
     return Either.left(new CookiesError({ reason: "InvalidValue" }))
   }
 
   if (options !== undefined) {
-    if (options.domain !== undefined && !fieldContentRegExp.test(options.domain)) {
+    if (options.domain !== undefined && !cookieDomainRegExp.test(options.domain)) {
       return Either.left(new CookiesError({ reason: "InvalidDomain" }))
     }
 
-    if (options.path !== undefined && !fieldContentRegExp.test(options.path)) {
+    if (options.path !== undefined && !cookiePathRegExp.test(options.path)) {
       return Either.left(new CookiesError({ reason: "InvalidPath" }))
     }
 

packages/platform/test/Cookies.test.ts

@@ -1,12 +1,54 @@
 import * as Cookies from "@effect/platform/Cookies"
 import { describe, it } from "@effect/vitest"
-import { throws } from "@effect/vitest/utils"
+import { deepStrictEqual, throws } from "@effect/vitest/utils"
+import { Either, Option } from "effect"
 
 const assertCookieError = (f: () => void, message: Cookies.CookiesError["reason"]) => {
   throws(f, new Cookies.CookiesError({ reason: message }))
 }
 
 describe("Cookies", () => {
+  it("makeCookie rejects cookie attribute separators", () => {
+    deepStrictEqual(
+      Cookies.makeCookie("name; HttpOnly", "value"),
+      Either.left(new Cookies.CookiesError({ reason: "InvalidName" }))
+    )
+    deepStrictEqual(
+      Cookies.makeCookie("name", "value", { domain: "example.com; Domain=evil.com" }),
+      Either.left(new Cookies.CookiesError({ reason: "InvalidDomain" }))
+    )
+    deepStrictEqual(
+      Cookies.makeCookie("name", "value", { path: "/; HttpOnly" }),
+      Either.left(new Cookies.CookiesError({ reason: "InvalidPath" }))
+    )
+  })
+
+  it("makeCookie accepts RFC 6265 cookie fields", () => {
+    const name = "!#$%&'*+-.^_`|~0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz"
+    for (const domain of ["api.example.com", ".api.example.com"]) {
+      const cookie = Either.getOrThrow(Cookies.makeCookie(name, "value", {
+        domain,
+        path: "/docs/a-b_c~d/%20"
+      }))
+
+      deepStrictEqual(
+        Cookies.serializeCookie(cookie),
+        `${name}=value; Domain=${domain}; Path=/docs/a-b_c~d/%20`
+      )
+    }
+  })
+
+  it("fromSetCookie validates cookie names", () => {
+    deepStrictEqual(Cookies.fromSetCookie("name; HttpOnly=value"), Cookies.empty)
+    deepStrictEqual(Cookies.fromSetCookie("name()=value"), Cookies.empty)
+
+    const name = "!#$%&'*+-.^_`|~"
+    deepStrictEqual(
+      Cookies.getValue(Cookies.fromSetCookie(`${name}=value`), name),
+      Option.some("value")
+    )
+  })
+
   it("unsafeMakeCookie", () => {
     assertCookieError(() => Cookies.unsafeMakeCookie("", "value"), "InvalidName")
     assertCookieError(() => Cookies.unsafeMakeCookie("name", "value", { domain: "" }), "InvalidDomain")