Sanitize cookie value before Set-Cookie interpolation

crates/common/src/cookies.rs

@@ -59,12 +59,34 @@ pub fn handle_request_cookies(
     }
 }
 
-/// Creates a synthetic ID cookie string.
+/// Returns `true` if every byte in `value` is a valid RFC 6265 `cookie-octet`.
+/// An empty string is always rejected.
 ///
-/// Generates a properly formatted cookie with security attributes
-/// for storing the synthetic ID.
+/// RFC 6265 restricts cookie values to printable US-ASCII excluding whitespace,
+/// double-quote, comma, semicolon, and backslash. Rejecting these characters
+/// prevents header-injection attacks where a crafted value could append
+/// spurious cookie attributes (e.g. `evil; Domain=.attacker.com`).
+///
+/// Non-ASCII characters (multi-byte UTF-8) are always rejected because their
+/// byte values exceed `0x7E`.
+#[must_use]
+fn is_safe_cookie_value(value: &str) -> bool {
+    // RFC 6265 §4.1.1 cookie-octet:
+    //   0x21        — '!'
+    //   0x23–0x2B  — '#' through '+'   (excludes 0x22 DQUOTE)
+    //   0x2D–0x3A  — '-' through ':'   (excludes 0x2C comma)
+    //   0x3C–0x5B  — '<' through '['   (excludes 0x3B semicolon)
+    //   0x5D–0x7E  — ']' through '~'   (excludes 0x5C backslash, 0x7F DEL)
+    // All control characters (0x00–0x20) and non-ASCII (0x80+) are also excluded.
+    !value.is_empty()
+        && value
+            .bytes()
+            .all(|b| matches!(b, 0x21 | 0x23..=0x2B | 0x2D..=0x3A | 0x3C..=0x5B | 0x5D..=0x7E))
+}
+
+/// Formats the `Set-Cookie` header value for the synthetic ID cookie.
 #[must_use]
-pub fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> String {
+fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> String {
     format!(
         "{}={}; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}",
         COOKIE_SYNTHETIC_ID, synthetic_id, settings.publisher.cookie_domain, COOKIE_MAX_AGE,
@@ -73,13 +95,24 @@ pub fn create_synthetic_cookie(settings: &Settings, synthetic_id: &str) -> Strin
 
 /// Sets the synthetic ID cookie on the given response.
 ///
-/// This helper abstracts the logic of creating the cookie string and appending
-/// the Set-Cookie header to the response.
+/// Validates `synthetic_id` against RFC 6265 `cookie-octet` rules before
+/// interpolation. If the value contains unsafe characters (e.g. semicolons),
+/// the cookie is not set and a warning is logged. This prevents an attacker
+/// from injecting spurious cookie attributes via a controlled ID value.
+///
+/// `cookie_domain` comes from operator configuration and is considered trusted.
 pub fn set_synthetic_cookie(
     settings: &Settings,
     response: &mut fastly::Response,
     synthetic_id: &str,
 ) {
+    if !is_safe_cookie_value(synthetic_id) {
+        log::warn!(
+            "Rejecting synthetic_id for Set-Cookie: value of {} bytes contains characters illegal in a cookie value",
+            synthetic_id.len()
+        );
+        return;
+    }
     response.append_header(
         header::SET_COOKIE,
         create_synthetic_cookie(settings, synthetic_id),
@@ -112,7 +145,7 @@ mod tests {
     }
 
     #[test]
-    fn test_parse_cookies_to_jar_emtpy() {
+    fn test_parse_cookies_to_jar_empty() {
         let cookie_str = "";
         let jar = parse_cookies_to_jar(cookie_str);
 
@@ -168,35 +201,102 @@ mod tests {
     }
 
     #[test]
-    fn test_create_synthetic_cookie() {
+    fn test_set_synthetic_cookie() {
         let settings = create_test_settings();
-        let result = create_synthetic_cookie(&settings, "12345");
+        let mut response = fastly::Response::new();
+        set_synthetic_cookie(&settings, &mut response, "abc123.XyZ789");
+
+        let cookie_str = response
+            .get_header(header::SET_COOKIE)
+            .expect("Set-Cookie header should be present")
+            .to_str()
+            .expect("header should be valid UTF-8");
+
         assert_eq!(
-            result,
+            cookie_str,
             format!(
-                "{}=12345; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}",
+                "{}=abc123.XyZ789; Domain={}; Path=/; Secure; SameSite=Lax; Max-Age={}",
                 COOKIE_SYNTHETIC_ID, settings.publisher.cookie_domain, COOKIE_MAX_AGE,
-            )
+            ),
+            "Set-Cookie header should match expected format"
         );
     }
 
     #[test]
-    fn test_set_synthetic_cookie() {
+    fn test_set_synthetic_cookie_rejects_semicolon() {
         let settings = create_test_settings();
         let mut response = fastly::Response::new();
-        set_synthetic_cookie(&settings, &mut response, "test-id-123");
+        set_synthetic_cookie(&settings, &mut response, "evil; Domain=.attacker.com");
 
-        let cookie_header = response
-            .get_header(header::SET_COOKIE)
-            .expect("Set-Cookie header should be present");
-        let cookie_str = cookie_header
-            .to_str()
-            .expect("header should be valid UTF-8");
+        assert!(
+            response.get_header(header::SET_COOKIE).is_none(),
+            "Set-Cookie should not be set when value contains a semicolon"
+        );
+    }
 
-        let expected = create_synthetic_cookie(&settings, "test-id-123");
-        assert_eq!(
-            cookie_str, expected,
-            "Set-Cookie header should match create_synthetic_cookie output"
+    #[test]
+    fn test_set_synthetic_cookie_rejects_crlf() {
+        let settings = create_test_settings();
+        let mut response = fastly::Response::new();
+        set_synthetic_cookie(&settings, &mut response, "evil\r\nX-Injected: header");
+
+        assert!(
+            response.get_header(header::SET_COOKIE).is_none(),
+            "Set-Cookie should not be set when value contains CRLF"
+        );
+    }
+
+    #[test]
+    fn test_set_synthetic_cookie_rejects_space() {
+        let settings = create_test_settings();
+        let mut response = fastly::Response::new();
+        set_synthetic_cookie(&settings, &mut response, "bad value");
+
+        assert!(
+            response.get_header(header::SET_COOKIE).is_none(),
+            "Set-Cookie should not be set when value contains whitespace"
+        );
+    }
+
+    #[test]
+    fn test_is_safe_cookie_value_rejects_empty_string() {
+        assert!(!is_safe_cookie_value(""), "should reject empty string");
+    }
+
+    #[test]
+    fn test_is_safe_cookie_value_accepts_valid_synthetic_id_characters() {
+        // Hex digits, dot separator, alphanumeric suffix — the full synthetic ID character set
+        assert!(
+            is_safe_cookie_value("abcdef0123456789.ABCDEFabcdef"),
+            "should accept hex digits, dots, and alphanumeric characters"
+        );
+    }
+
+    #[test]
+    fn test_is_safe_cookie_value_rejects_non_ascii() {
+        assert!(
+            !is_safe_cookie_value("valüe"),
+            "should reject non-ASCII UTF-8 characters"
+        );
+    }
+
+    #[test]
+    fn test_is_safe_cookie_value_rejects_illegal_characters() {
+        assert!(!is_safe_cookie_value("val;ue"), "should reject semicolon");
+        assert!(!is_safe_cookie_value("val,ue"), "should reject comma");
+        assert!(
+            !is_safe_cookie_value("val\"ue"),
+            "should reject double-quote"
+        );
+        assert!(!is_safe_cookie_value("val\\ue"), "should reject backslash");
+        assert!(!is_safe_cookie_value("val ue"), "should reject space");
+        assert!(
+            !is_safe_cookie_value("val\x00ue"),
+            "should reject null byte"
+        );
+        assert!(
+            !is_safe_cookie_value("val\x7fue"),
+            "should reject DEL character"
         );
     }
 }

crates/common/src/integrations/registry.rs

@@ -658,7 +658,9 @@ impl IntegrationRegistry {
             // Generate synthetic ID before consuming request
             let synthetic_id_result = get_or_generate_synthetic_id(settings, &req);
 
-            // Set synthetic ID header on the request so integrations can read it
+            // Set synthetic ID header on the request so integrations can read it.
+            // Header injection: Fastly's HeaderValue API rejects values containing \r, \n, or \0,
+            // so a crafted synthetic_id cannot inject additional request headers.
             if let Ok(ref synthetic_id) = synthetic_id_result {
                 req.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str());
             }
@@ -669,7 +671,13 @@ impl IntegrationRegistry {
             if let Ok(ref mut response) = result {
                 match synthetic_id_result {
                     Ok(ref synthetic_id) => {
+                        // Response-header injection: Fastly's HeaderValue API rejects values
+                        // containing \r, \n, or \0, so a crafted synthetic_id cannot inject
+                        // additional response headers.
                         response.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str());
+                        // Cookie is intentionally not set when synthetic_id contains RFC 6265-illegal
+                        // characters (e.g. a crafted x-synthetic-id header value). The response header
+                        // is still emitted; only cookie persistence is skipped.
                         set_synthetic_cookie(settings, response, synthetic_id.as_str());
                     }
                     Err(ref err) => {

crates/common/src/publisher.rs

@@ -334,7 +334,12 @@ pub fn handle_publisher_request(
         );
     }
 
+    // Response-header injection: Fastly's HeaderValue API rejects values containing \r, \n,
+    // or \0, so a crafted synthetic_id cannot inject additional response headers.
     response.set_header(HEADER_X_SYNTHETIC_ID, synthetic_id.as_str());
+    // Cookie is intentionally not set when synthetic_id contains RFC 6265-illegal characters
+    // (e.g. a crafted x-synthetic-id header value). The response header is still emitted so
+    // upstream components see the ID; only cookie persistence is skipped.
     set_synthetic_cookie(settings, &mut response, synthetic_id.as_str());
 
     Ok(response)

crates/common/src/synthetic.rs

@@ -132,6 +132,10 @@ pub fn get_synthetic_id(req: &Request) -> Result<Option<String>, Report<TrustedS
         .and_then(|h| h.to_str().ok())
     {
         let id = synthetic_id.to_string();
+        // The ID is user-supplied from the x-synthetic-id request header. Log injection via
+        // embedded newlines is not a concern here: Fastly's log-fastly backend transmits log
+        // records as opaque byte sequences, so a newline in a field value does not synthesise
+        // an additional log record at the collector.
         log::info!("Using existing Synthetic ID from header: {}", id);
         return Ok(Some(id));
     }
@@ -140,6 +144,9 @@ pub fn get_synthetic_id(req: &Request) -> Result<Option<String>, Report<TrustedS
         Some(jar) => {
             if let Some(cookie) = jar.get(COOKIE_SYNTHETIC_ID) {
                 let id = cookie.value().to_string();
+                // The ID is user-supplied from the synthetic_id cookie. The same log-injection
+                // note as the header path applies: Fastly's log-fastly backend does not treat
+                // embedded newlines as record separators.
                 log::info!("Using existing Trusted Server ID from cookie: {}", id);
                 return Ok(Some(id));
             }