From 8008d5aa8573a25bda53bdd372241d54ccca69da Mon Sep 17 00:00:00 2001 From: Sebastian C Date: Wed, 30 Jul 2025 21:52:50 -0500 Subject: [PATCH] net: Add expiry limit to cookies and prevent panics from max-age (#38376) Based on RFC6256 expiration must be limited to 400 days. This also solves a bug I found where large max-age values could cause an overflow panic when adding. Testing: New unit test added. There doesn't appear to be a WPT test that relies on this under cookies/ but cookiestore/ does. Signed-off-by: Sebastian C --- components/net/cookie.rs | 15 ++++++++++++--- components/net/tests/cookie.rs | 10 ++++++++++ 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/components/net/cookie.rs b/components/net/cookie.rs index 986a02a07a3..a5cf597505e 100644 --- a/components/net/cookie.rs +++ b/components/net/cookie.rs @@ -21,7 +21,7 @@ use nom::multi::{many0, many1, separated_list1}; use nom::sequence::{delimited, preceded, terminated, tuple}; use serde::{Deserialize, Serialize}; use servo_url::ServoUrl; -use time::{Date, Month, OffsetDateTime, Time}; +use time::{Date, Duration, Month, OffsetDateTime, Time}; /// A stored cookie that wraps the definition in cookie-rs. This is used to implement /// various behaviours defined in the spec that rely on an associated request URL, @@ -84,18 +84,27 @@ impl ServoCookie { // 1. Set the cookie's persistent-flag to true. persistent = true; + // The user agent MUST limit the maximum value of the Max-Age attribute. + // The limit SHOULD NOT be greater than 400 days (34560000 seconds) in the future. + let clamped_max_age = max_age.min(Duration::seconds(34560000)); + // 2. Set the cookie's expiry-time to attribute-value of the last // attribute in the cookie-attribute-list with an attribute-name of "Max-Age". - expiry_time = Some(SystemTime::now() + max_age); + expiry_time = Some(SystemTime::now() + clamped_max_age); } // Otherwise, if the cookie-attribute-list contains an attribute with an attribute-name of "Expires": else if let Some(date_time) = cookie.expires_datetime() { // 1. Set the cookie's persistent-flag to true. persistent = true; + // The user agent MUST limit the maximum value of the Expires attribute. + // The limit SHOULD NOT be greater than 400 days (34560000 seconds) in the future. + let clamped_date_time = + date_time.min(OffsetDateTime::now_utc() + Duration::seconds(34560000)); + // 2. Set the cookie's expiry-time to attribute-value of the last attribute in the // cookie-attribute-list with an attribute-name of "Expires". - expiry_time = Some(date_time.into()); + expiry_time = Some(clamped_date_time.into()); } // Otherwise: else { diff --git a/components/net/tests/cookie.rs b/components/net/tests/cookie.rs index 76b1760f2a7..d9de36312df 100644 --- a/components/net/tests/cookie.rs +++ b/components/net/tests/cookie.rs @@ -2,6 +2,8 @@ * License, v. 2.0. If a copy of the MPL was not distributed with this * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ +use std::time::{Duration, SystemTime}; + use net::cookie::ServoCookie; use net::cookie_storage::CookieStorage; use net_traits::CookieSource; @@ -101,6 +103,14 @@ fn fn_cookie_constructor() { let u = &ServoUrl::parse("http://example.com/foobar").unwrap(); let cookie = cookie::Cookie::parse("foobar=value;path=/").unwrap(); assert!(ServoCookie::new_wrapped(cookie, u, CookieSource::HTTP).is_some()); + + let cookie = cookie::Cookie::parse("foo=bar; max-age=99999999999999999999999999999").unwrap(); + let cookie = ServoCookie::new_wrapped(cookie, u, CookieSource::HTTP).unwrap(); + assert!( + cookie + .expiry_time + .is_some_and(|exp| exp < SystemTime::now() + Duration::from_secs(401 * 24 * 60 * 60)) + ); } #[test]