fshttp: don't send --header values to other hosts on redirect GHSA-486v-q2wf-fp2r CVE-PENDING

The headers set with --header and --header-download are added to
every request by the rclone transport, including redirect hops which
net/http makes to other hosts, so a credential passed with --header
for one host could be sent to any host that server chose to redirect
to.

The transport now walks the redirect chain net/http records on each
redirected request and, once the chain has visited a host other than
the one originally requested, removes the headers rather than adding
them.

Also restore the global --client-cert and --client-key config after
TestCertificates so its temporary files are not used by later tests.

fs/fshttp/http.go

@@ -25,6 +25,7 @@ import (
 	"github.com/rclone/rclone/fs"
 	"github.com/rclone/rclone/fs/accounting"
 	"github.com/rclone/rclone/fs/config/obscure"
+	"github.com/rclone/rclone/lib/rest"
 	"github.com/rclone/rclone/lib/structs"
 	"github.com/youmark/pkcs8"
 	"golang.org/x/net/publicsuffix"
@@ -591,6 +592,22 @@ func newClientTrace(req *http.Request) *httptrace.ClientTrace {
 	}
 }
 
+// redirectLeavesHost reports whether req is a redirect hop in a
+// chain which has visited a host other than that of the original
+// request at any point.
+func redirectLeavesHost(req *http.Request) bool {
+	origin := req
+	for origin.Response != nil && origin.Response.Request != nil {
+		origin = origin.Response.Request
+	}
+	for hop := req; hop != origin; hop = hop.Response.Request {
+		if !rest.SameHost(hop.URL, origin.URL) {
+			return true
+		}
+	}
+	return false
+}
+
 // RoundTrip implements the RoundTripper interface.
 func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error) {
 	// Check if certificates are being used and the certificates are expired
@@ -602,9 +619,15 @@ func (t *Transport) RoundTrip(req *http.Request) (resp *http.Response, err error
 	accounting.LimitTPS(req.Context())
 	// Force user agent
 	req.Header.Set("User-Agent", t.userAgent)
-	// Set user defined headers
-	for _, option := range t.headers {
-		req.Header.Set(option.Key, option.Value)
+	// Set user defined headers, unless redirected elsewhere
+	if redirectLeavesHost(req) {
+		for _, option := range t.headers {
+			req.Header.Del(option.Key)
+		}
+	} else {
+		for _, option := range t.headers {
+			req.Header.Set(option.Key, option.Value)
+		}
 	}
 	// Filter the request if required
 	if t.filterRequest != nil {

fs/fshttp/http_test.go

@@ -174,6 +174,12 @@ func TestCertificates(t *testing.T) {
 	// create a test cert/key pair and write it to the files
 	ctx := context.TODO()
 	ci := fs.GetConfig(ctx)
+	// Restore the global config for later tests as the temp files
+	// are removed when this test finishes
+	oldCert, oldKey := ci.ClientCert, ci.ClientKey
+	t.Cleanup(func() {
+		ci.ClientCert, ci.ClientKey = oldCert, oldKey
+	})
 	// Create a test certificate and write it to a temp file
 	ci.ClientCert = t.TempDir() + "client.cert"
 	ci.ClientKey = t.TempDir() + "client.key"
@@ -202,3 +208,59 @@ func TestCertificates(t *testing.T) {
 	_, err = client.Get(ts.URL)
 	assert.NoError(t, err)
 }
+
+// TestRedirectStripsGlobalHeaders checks the headers set with --header
+// are sent to the requested host and any redirect on it but not to
+// another host the request is redirected to
+func TestRedirectStripsGlobalHeaders(t *testing.T) {
+	var got http.Header
+	target := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		got = r.Header.Clone()
+	}))
+	defer target.Close()
+
+	var ts *httptest.Server
+	ts = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch r.URL.Path {
+		case "/same":
+			http.Redirect(w, r, ts.URL+"/moved", http.StatusFound)
+		case "/other":
+			http.Redirect(w, r, target.URL+"/moved", http.StatusFound)
+		case "/back":
+			// Chain via the other host and back again
+			http.Redirect(w, r, target.URL+"/bounce", http.StatusFound)
+		default:
+			got = r.Header.Clone()
+		}
+	}))
+	defer ts.Close()
+	// The other host bounces /bounce back to the original host
+	target.Config.Handler = http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if r.URL.Path == "/bounce" {
+			http.Redirect(w, r, ts.URL+"/moved", http.StatusFound)
+			return
+		}
+		got = r.Header.Clone()
+	})
+
+	ctx, ci := fs.AddConfig(context.Background())
+	ci.Headers = []*fs.HTTPOption{{Key: "X-Potato", Value: "sausage"}}
+	client := NewClient(ctx)
+
+	for _, test := range []struct {
+		path string
+		want string
+	}{
+		{"/direct", "sausage"},
+		{"/same", "sausage"},
+		{"/other", ""},
+		{"/back", ""},
+	} {
+		got = nil
+		resp, err := client.Get(ts.URL + test.path)
+		require.NoError(t, err, test.path)
+		require.NoError(t, resp.Body.Close())
+		require.NotNil(t, got, test.path)
+		assert.Equal(t, test.want, got.Get("X-Potato"), test.path)
+	}
+}