From fbae1391ac1b0ad28405be6a547acd0879a1830a Mon Sep 17 00:00:00 2001
From: Alec Moskvin <alecm@gmx.com>
Date: Wed, 14 Jun 2023 00:17:50 -0400
Subject: [PATCH] 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.
---
 src/test/redirect.rs | 44 ++++++++++++++++++++++++++++++++++++++++++++
 src/unit.rs          |  6 ++++--
 2 files changed, 48 insertions(+), 2 deletions(-)

diff --git a/src/test/redirect.rs b/src/test/redirect.rs
index ed50008..ef5ff63 100644
--- a/src/test/redirect.rs
+++ b/src/test/redirect.rs
@@ -163,6 +163,50 @@ fn redirect_post_with_data() {
     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]
 fn redirect_308() {
     test::set_handler("/redirect_get3", |_| {
diff --git a/src/unit.rs b/src/unit.rs
index dffc896..13c3c9a 100644
--- a/src/unit.rs
+++ b/src/unit.rs
@@ -217,9 +217,11 @@ pub(crate) fn connect(
         let mut headers = unit.headers;
 
         // 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| {
-            !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.