Auto merge of #14491 - KiChjang:cookie-checks, r=jdm

Add domain and path checks for secure cookies eviction

Fixes #14477.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14491)
<!-- Reviewable:end -->

components/net/cookie.rs

@@ -75,7 +75,7 @@ impl Cookie {
 
 
         // Step 10
-        if cookie.httponly && source != CookieSource::HTTP {
+        if cookie.httponly && source == CookieSource::NonHTTP {
             return None;
         }
 
@@ -132,16 +132,11 @@ impl Cookie {
 
     // http://tools.ietf.org/html/rfc6265#section-5.1.3
     pub fn domain_match(string: &str, domain_string: &str) -> bool {
-        if string == domain_string {
-            return true;
-        }
-        if string.ends_with(domain_string) &&
-            string.as_bytes()[string.len()-domain_string.len()-1] == b'.' &&
-            string.parse::<Ipv4Addr>().is_err() &&
-            string.parse::<Ipv6Addr>().is_err() {
-            return true;
-        }
-        false
+        string == domain_string ||
+        (string.ends_with(domain_string) &&
+         string.as_bytes()[string.len()-domain_string.len()-1] == b'.' &&
+         string.parse::<Ipv4Addr>().is_err() &&
+         string.parse::<Ipv6Addr>().is_err())
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.4 step 1

components/net/cookie_storage.rs

@@ -33,11 +33,32 @@ impl CookieStorage {
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.3
-    pub fn remove(&mut self, cookie: &Cookie, source: CookieSource) -> Result<Option<Cookie>, ()> {
+    pub fn remove(&mut self, cookie: &Cookie, url: &ServoUrl, source: CookieSource) -> Result<Option<Cookie>, ()> {
         let domain = reg_host(cookie.cookie.domain.as_ref().unwrap_or(&"".to_string()));
         let cookies = self.cookies_map.entry(domain).or_insert(vec![]);
 
-        // Step 1
+        // https://www.ietf.org/id/draft-ietf-httpbis-cookie-alone-01.txt Step 2
+        if !cookie.cookie.secure && url.scheme() != "https" && url.scheme() != "wss" {
+            let new_domain = cookie.cookie.domain.as_ref().unwrap();
+            let new_path = cookie.cookie.path.as_ref().unwrap();
+
+            let any_overlapping = cookies.iter().any(|c| {
+                let existing_domain = c.cookie.domain.as_ref().unwrap();
+                let existing_path = c.cookie.path.as_ref().unwrap();
+
+                c.cookie.name == cookie.cookie.name &&
+                c.cookie.secure &&
+                (Cookie::domain_match(new_domain, existing_domain) ||
+                 Cookie::domain_match(existing_domain, new_domain)) &&
+                Cookie::path_match(new_path, existing_path)
+            });
+
+            if any_overlapping {
+                return Err(());
+            }
+        }
+
+        // Step 11.1
         let position = cookies.iter().position(|c| {
             c.cookie.domain == cookie.cookie.domain &&
             c.cookie.path == cookie.cookie.path &&
@@ -45,15 +66,16 @@ impl CookieStorage {
         });
 
         if let Some(ind) = position {
+            // Step 11.4
             let c = cookies.remove(ind);
 
             // http://tools.ietf.org/html/rfc6265#section-5.3 step 11.2
-            if !c.cookie.httponly || source == CookieSource::HTTP {
-                Ok(Some(c))
-            } else {
+            if c.cookie.httponly && source == CookieSource::NonHTTP {
                 // Undo the removal.
                 cookies.push(c);
                 Err(())
+            } else {
+                Ok(Some(c))
             }
         } else {
             Ok(None)
@@ -61,8 +83,13 @@ impl CookieStorage {
     }
 
     // http://tools.ietf.org/html/rfc6265#section-5.3
-    pub fn push(&mut self, mut cookie: Cookie, source: CookieSource) {
-        let old_cookie = self.remove(&cookie, source);
+    pub fn push(&mut self, mut cookie: Cookie, url: &ServoUrl, source: CookieSource) {
+        // https://www.ietf.org/id/draft-ietf-httpbis-cookie-alone-01.txt Step 1
+        if cookie.cookie.secure && url.scheme() != "https" && url.scheme() != "wss" {
+            return;
+        }
+
+        let old_cookie = self.remove(&cookie, url, source);
         if old_cookie.is_err() {
             // This new cookie is not allowed to overwrite an existing one.
             return;
@@ -83,7 +110,7 @@ impl CookieStorage {
             cookies.retain(|c| !is_cookie_expired(&c));
             let new_len = cookies.len();
 
-            // https://datatracker.ietf.org/doc/draft-ietf-httpbis-cookie-alone
+            // https://www.ietf.org/id/draft-ietf-httpbis-cookie-alone-01.txt
             if new_len == old_len && !evict_one_cookie(cookie.cookie.secure, cookies) {
                 return;
             }
@@ -119,7 +146,6 @@ impl CookieStorage {
             // Step 1
             c.appropriate_for_url(url, source)
         };
-
         // Step 2
         let domain = reg_host(url.host_str().unwrap_or(""));
         let cookies = self.cookies_map.entry(domain).or_insert(vec![]);
@@ -159,6 +185,7 @@ impl CookieStorage {
         }))
     }
 }
+
 fn reg_host<'a>(url: &'a str) -> String {
     reg_suffix(url).to_string()
 }

components/net/http_loader.rs

@@ -281,7 +281,7 @@ fn set_cookie_for_url(cookie_jar: &Arc<RwLock<CookieStorage>>,
     if let Ok(SetCookie(cookies)) = header {
         for bare_cookie in cookies {
             if let Some(cookie) = cookie::Cookie::new_wrapped(bare_cookie, request, source) {
-                cookie_jar.push(cookie, source);
+                cookie_jar.push(cookie, request, source);
             }
         }
     }

components/net/resource_thread.rs

@@ -312,7 +312,7 @@ impl CoreResourceManager {
                           resource_group: &ResourceGroup) {
         if let Some(cookie) = cookie::Cookie::new_wrapped(cookie, &request, source) {
             let mut cookie_jar = resource_group.cookie_jar.write().unwrap();
-            cookie_jar.push(cookie, source)
+            cookie_jar.push(cookie, request, source)
         }
     }