Fix cookie leak on redirect (#608)

This removes the cookie header on redirect, otherwise if there are
multiple redirects, a new cookie header gets appended each time, while
the existing one remains. This could result in the cookies being leaked
to a third party.
This commit is contained in:
Alec Moskvin
2023-06-14 00:17:50 -04:00
committed by GitHub
parent 329e1f4ada
commit fbae1391ac
2 changed files with 48 additions and 2 deletions

View File

@@ -163,6 +163,50 @@ fn redirect_post_with_data() {
assert_eq!(resp.status(), 200); assert_eq!(resp.status(), 200);
} }
#[cfg(feature = "cookies")]
#[test]
fn redirect_post_with_cookies() {
test::set_handler("/secure/login", |_| {
test::make_response(
302,
"Go here",
vec![
"Set-cookie: USER=ferris; Path=/; SameSite=Strict",
"Set-cookie: SECRET=123456; Path=/secure; SameSite=Strict",
"Location: /secure/login-handler",
],
vec![],
)
});
test::set_handler("/secure/login-handler", |unit| {
let cookie_headers = unit.all("cookie");
assert_eq!(cookie_headers.len(), 1);
assert!(cookie_headers.first().unwrap().contains("USER"));
assert!(cookie_headers.first().unwrap().contains("SECRET"));
test::make_response(302, "Go here", vec!["Location: /out"], vec![])
});
test::set_handler("/out", |unit| {
let cookie_headers = unit.all("cookie");
assert_eq!(unit.all("cookie").len(), 1);
assert!(cookie_headers.first().unwrap().contains("USER"));
assert!(!cookie_headers.first().unwrap().contains("SECRET"));
test::make_response(
302,
"Go away",
vec!["Location: test://external.example/external"],
vec![],
)
});
test::set_handler("/external", |unit| {
assert_eq!(unit.method, "GET");
assert_eq!(unit.all("cookie").len(), 0);
test::make_response(200, "OK", vec![], vec![])
});
let resp = post("test://host.example/secure/login").call().unwrap();
assert_eq!(resp.status(), 200);
assert_eq!(resp.get_url(), "test://external.example/external");
}
#[test] #[test]
fn redirect_308() { fn redirect_308() {
test::set_handler("/redirect_get3", |_| { test::set_handler("/redirect_get3", |_| {

View File

@@ -217,9 +217,11 @@ pub(crate) fn connect(
let mut headers = unit.headers; let mut headers = unit.headers;
// on redirects we don't want to keep "content-length". we also might want to // on redirects we don't want to keep "content-length". we also might want to
// strip away "authorization" to ensure credentials are not leaked. // strip away "authorization" and "cookie" to ensure credentials are not leaked.
headers.retain(|h| { headers.retain(|h| {
!h.is_name("content-length") && (!h.is_name("authorization") || keep_auth_header) !h.is_name("content-length")
&& !h.is_name("cookie")
&& (!h.is_name("authorization") || keep_auth_header)
}); });
// recreate the unit to get a new hostname and cookies for the new host. // recreate the unit to get a new hostname and cookies for the new host.