send -H cookies once on same-host redirects

common/httpx/httpx.go

@@ -98,9 +98,8 @@ func New(options *Options) (*HTTPX, error) {
 			}
 
 			// Custom cookies are kept only for the same host (or its subdomains)
-			if isSameHostOrSubdomain(newHost, initialHost) {
-				httpx.setCustomCookies(redirectedRequest)
-			} else {
+			sameScope := isSameHostOrSubdomain(newHost, initialHost)
+			if !sameScope {
 				redirectedRequest.Header.Del("Cookie")
 			}
 
@@ -122,6 +121,12 @@ func New(options *Options) (*HTTPX, error) {
 				}
 			}
 
+			// After the secret cleanup, which removes cookies by name, so a -H
+			// cookie that shares a name with a removed secret cookie comes back
+			if sameScope {
+				httpx.setCustomCookies(redirectedRequest)
+			}
+
 			if options.RespectHSTS {
 				handleHSTS(redirectedRequest)
 			}
@@ -662,11 +667,21 @@ func (h *HTTPX) SetCustomHeaders(r *retryablehttp.Request, headers map[string][]
 	}
 }
 
+// setCustomCookies adds the -H cookies to a redirect hop. On a same-host hop
+// net/http has already copied them, so cookies the request has are skipped.
 func (httpx *HTTPX) setCustomCookies(req *http.Request) {
-	if httpx.Options.hasCustomCookies() {
-		for _, cookie := range httpx.Options.customCookies {
-			req.AddCookie(cookie)
+	if !httpx.Options.hasCustomCookies() {
+		return
+	}
+	present := make(map[string]struct{})
+	for _, cookie := range req.Cookies() {
+		present[cookie.Name] = struct{}{}
+	}
+	for _, cookie := range httpx.Options.customCookies {
+		if _, ok := present[cookie.Name]; ok {
+			continue
 		}
+		req.AddCookie(cookie)
 	}
 }
 

common/httpx/redirect_test.go

@@ -240,3 +240,61 @@ func TestRedirect_QueryAuth_TargetParamPreserved(t *testing.T) {
 	// Target token provided by Location redirect must be preserved
 	require.Equal(t, "target-token", targetQueryToken)
 }
+
+// A -H cookie that shares its name with a secret-file cookie must survive a
+// same-host redirect to a URL the secret does not cover: the secret cleanup
+// removes cookies by name, and the -H cookie has to come back after it.
+func TestRedirect_CustomCookie_KeptWhenSameNameSecretRemoved(t *testing.T) {
+	var targetCookies []*http.Cookie
+	targetServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		targetCookies = r.Cookies()
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer targetServer.Close()
+
+	initialServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, targetServer.URL, http.StatusFound)
+	}))
+	defer initialServer.Close()
+
+	// the secret covers only the initial server's host:port
+	mockAuth := &mockAuthProvider{
+		strategies: map[string][]authx.AuthStrategy{
+			initialServer.Listener.Addr().String(): {
+				authx.NewCookiesAuthStrategy(&authx.Secret{
+					Cookies: []authx.Cookie{{Key: "shared", Value: "SECRET-VALUE"}},
+				}),
+			},
+		},
+	}
+
+	options := DefaultOptions
+	options.FollowRedirects = true
+	options.Timeout = 5 * time.Second
+	options.AuthProvider = mockAuth
+	options.CustomHeaders = map[string][]string{
+		"Cookie": {"shared=H-VALUE"},
+	}
+
+	client, err := New(&options)
+	require.NoError(t, err)
+
+	req, err := retryablehttp.NewRequest("GET", initialServer.URL, nil)
+	require.NoError(t, err)
+	client.SetCustomHeaders(req, client.CustomHeaders)
+	for _, s := range mockAuth.LookupURL(req.URL.URL) {
+		s.ApplyOnRR(req)
+	}
+
+	resp, err := client.Do(req, UnsafeOptions{})
+	require.NoError(t, err)
+	require.NotNil(t, resp)
+
+	var values []string
+	for _, c := range targetCookies {
+		if c.Name == "shared" {
+			values = append(values, c.Value)
+		}
+	}
+	require.Equal(t, []string{"H-VALUE"}, values, "redirect target cookies: %v", targetCookies)
+}

runner/redirect_credentials_test.go

@@ -0,0 +1,151 @@
+package runner
+
+import (
+	"fmt"
+	"io"
+	"net"
+	"net/http"
+	"net/http/httptest"
+	"os"
+	"path/filepath"
+	"strings"
+	"sync"
+	"testing"
+
+	"github.com/projectdiscovery/goflags"
+	"github.com/projectdiscovery/httpx/common/customheader"
+	"github.com/stretchr/testify/require"
+)
+
+type headerLog struct {
+	mu      sync.Mutex
+	headers map[string]http.Header
+}
+
+func (l *headerLog) record(path string, h http.Header) {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	if l.headers == nil {
+		l.headers = make(map[string]http.Header)
+	}
+	l.headers[path] = h.Clone()
+}
+
+func (l *headerLog) get(path string) http.Header {
+	l.mu.Lock()
+	defer l.mu.Unlock()
+	return l.headers[path]
+}
+
+// TestRedirectCredentialsStayOnHost checks that -fr does not send -H cookies
+// or secret-file headers to another host, and keeps them on the same host.
+func TestRedirectCredentialsStayOnHost(t *testing.T) {
+	log := &headerLog{}
+
+	// [::1] is another host than the 127.0.0.1 target and needs no name resolution
+	ln, err := net.Listen("tcp6", "[::1]:0")
+	if err != nil {
+		t.Skipf("ipv6 loopback is not available: %v", err)
+	}
+	var targetURL string
+	third := httptest.NewUnstartedServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/to-target" {
+			http.Redirect(w, r, targetURL+"/inbound", http.StatusFound)
+			return
+		}
+		log.record("third"+r.URL.Path, r.Header)
+		_, _ = io.WriteString(w, "third")
+	}))
+	_ = third.Listener.Close()
+	third.Listener = ln
+	third.Start()
+	defer third.Close()
+	thirdURL := third.URL
+
+	mux := http.NewServeMux()
+	mux.HandleFunc("/away", func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, thirdURL+"/landing", http.StatusFound)
+	})
+	mux.HandleFunc("/stay", func(w http.ResponseWriter, r *http.Request) {
+		http.Redirect(w, r, "/final", http.StatusFound)
+	})
+	mux.HandleFunc("/final", func(w http.ResponseWriter, r *http.Request) {
+		log.record("target/final", r.Header)
+		_, _ = io.WriteString(w, "final")
+	})
+	mux.HandleFunc("/inbound", func(w http.ResponseWriter, r *http.Request) {
+		log.record("target/inbound", r.Header)
+		_, _ = io.WriteString(w, "inbound")
+	})
+	target := httptest.NewServer(mux)
+	defer target.Close()
+	targetURL = target.URL
+
+	targetHost := strings.TrimPrefix(target.URL, "http://")
+	secrets := filepath.Join(t.TempDir(), "secrets.yaml")
+	require.NoError(t, os.WriteFile(secrets, []byte(fmt.Sprintf(`id: test
+info:
+  name: test
+static:
+  - type: Header
+    domains:
+      - %s
+    headers:
+      - key: X-Api-Key
+        value: SECRET-SF-KEY
+  - type: Cookie
+    domains:
+      - %s
+    cookies:
+      - key: sfsess
+        value: SECRET-SF-COOKIE
+`, targetHost, targetHost)), 0o600))
+
+	var mu sync.Mutex
+	var results []Result
+	options := &Options{
+		Methods:         "GET",
+		InputTargetHost: goflags.StringSlice{target.URL + "/away", target.URL + "/stay", thirdURL + "/to-target"},
+		FollowRedirects: true,
+		MaxRedirects:    10,
+		SecretFile:      secrets,
+		CustomHeaders:   customheader.CustomHeaders{"Cookie: sess=SECRET-H-COOKIE"},
+		Threads:         1,
+		Timeout:         5,
+		OnResult: func(r Result) {
+			mu.Lock()
+			results = append(results, r)
+			mu.Unlock()
+		},
+	}
+	require.NoError(t, options.ValidateOptions())
+	r, err := New(options)
+	require.NoError(t, err)
+	r.RunEnumeration()
+	r.Close()
+
+	for _, res := range results {
+		require.NoError(t, res.Err, res.Input)
+	}
+
+	got := log.get("third/landing")
+	require.NotNil(t, got, "redirect to the other host was not followed")
+	require.Empty(t, got.Get("Cookie"), "-H cookie sent to another host")
+	require.Empty(t, got.Get("X-Api-Key"), "secret-file header sent to a host it is not scoped to")
+
+	got = log.get("target/final")
+	require.NotNil(t, got, "same-host redirect was not followed")
+	require.Equal(t, "SECRET-SF-KEY", got.Get("X-Api-Key"))
+	cookies := (&http.Request{Header: got}).Cookies()
+	counts := map[string]int{}
+	for _, c := range cookies {
+		counts[c.Name]++
+	}
+	require.Equal(t, 1, counts["sess"], "-H cookie on a same-host redirect: %v", got.Values("Cookie"))
+	require.Equal(t, 1, counts["sfsess"], "secret-file cookie on a same-host redirect: %v", got.Values("Cookie"))
+
+	// the first URL matches no secret, the redirect host does
+	got = log.get("target/inbound")
+	require.NotNil(t, got, "redirect into the secret host was not followed")
+	require.Equal(t, "SECRET-SF-KEY", got.Get("X-Api-Key"), "secret-file header missing after a redirect into its host")
+}