fix(net): match cookie Path on a / boundary (RFC 6265) (#383)
Cookie path filtering used `request_path.starts_with(cookie_path)`, so a Path=/admin cookie was sent to sibling paths that merely share the string prefix, such as /administrator or /admin-backup. Per RFC 6265 5.1.4 a prefix match also requires the boundary to fall on a '/'. Add a path_matches helper (identical path, or a prefix ending in '/', or a prefix followed by '/') and use it in get_cookie_header and get_js_visible_cookies. Fixes #382.
This commit is contained in:
1 parent
c2b79b67fe
commit
d4fd2884a2
1 file changed
+40
-2
@@ -174,7 +174,7 @@ impl CookieJar {
|
||||
if entry.secure && !is_secure {
|
||||
continue;
|
||||
}
|
||||
if !path.starts_with(&entry.path) {
|
||||
if !path_matches(path, &entry.path) {
|
||||
continue;
|
||||
}
|
||||
matching.push(format!("{}={}", entry.name, entry.value));
|
||||
@@ -262,7 +262,7 @@ impl CookieJar {
|
||||
if entry.secure && !is_secure {
|
||||
continue;
|
||||
}
|
||||
if !path.starts_with(&entry.path) {
|
||||
if !path_matches(path, &entry.path) {
|
||||
continue;
|
||||
}
|
||||
matching.push(format!("{}={}", entry.name, entry.value));
|
||||
@@ -567,6 +567,21 @@ fn resolve_cookie_domain(origin_host: &str, domain_attr: Option<&str>) -> Option
|
||||
}
|
||||
}
|
||||
|
||||
// RFC 6265 5.1.4 path-match. A bare `starts_with` over-matches sibling paths
|
||||
// that share a string prefix (a Path=/admin cookie leaking to /administrator),
|
||||
// so a prefix match also requires the boundary to fall on a '/'.
|
||||
fn path_matches(request_path: &str, cookie_path: &str) -> bool {
|
||||
if request_path == cookie_path {
|
||||
return true;
|
||||
}
|
||||
if !request_path.starts_with(cookie_path) {
|
||||
return false;
|
||||
}
|
||||
// Prefix match: exact when the cookie-path already ends in '/', otherwise
|
||||
// the next char of the request-path must be the '/' boundary.
|
||||
cookie_path.ends_with('/') || request_path.as_bytes().get(cookie_path.len()) == Some(&b'/')
|
||||
}
|
||||
|
||||
fn domain_matches(host: &str, domain: &str) -> bool {
|
||||
// Avoid allocations on the hot path. Cookie lookup runs per fetch
|
||||
// (every subresource on a page) and walks every domain in the jar.
|
||||
@@ -948,4 +963,27 @@ mod tests {
|
||||
let api = Url::parse("http://api.example.com/").unwrap();
|
||||
assert!(jar.get_cookie_header(&api).contains("token=1"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn cookie_path_requires_slash_boundary() {
|
||||
// RFC 6265 5.1.4: a Path=/admin cookie must NOT be sent to a sibling
|
||||
// path like /administrator that merely shares the string prefix. It is
|
||||
// sent to /admin, /admin/, and /admin/x.
|
||||
let jar = CookieJar::new();
|
||||
let admin = Url::parse("https://example.com/admin").unwrap();
|
||||
jar.set_cookie("sess=1; Path=/admin", &admin);
|
||||
|
||||
let sibling = Url::parse("https://example.com/administrator").unwrap();
|
||||
assert!(
|
||||
!jar.get_cookie_header(&sibling).contains("sess=1"),
|
||||
"cookie leaked to sibling path /administrator: {}",
|
||||
jar.get_cookie_header(&sibling)
|
||||
);
|
||||
|
||||
assert!(jar.get_cookie_header(&admin).contains("sess=1"));
|
||||
let exact_slash = Url::parse("https://example.com/admin/").unwrap();
|
||||
assert!(jar.get_cookie_header(&exact_slash).contains("sess=1"));
|
||||
let sub = Url::parse("https://example.com/admin/panel").unwrap();
|
||||
assert!(jar.get_cookie_header(&sub).contains("sess=1"));
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user