diff --git a/backend/s3/s3.go b/backend/s3/s3.go index eb5e879c2..a2da0a0b5 100644 --- a/backend/s3/s3.go +++ b/backend/s3/s3.go @@ -1370,13 +1370,13 @@ func s3RedirectCrossesHost(req *http.Request, via []*http.Request) bool { if len(via) == 0 { return false } - host := via[0].URL.Host + scheme, host := via[0].URL.Scheme, via[0].URL.Host for _, redirect := range via[1:] { - if redirect.URL.Host != host { + if redirect.URL.Host != host || redirect.URL.Scheme != scheme { return true } } - return host != req.URL.Host + return host != req.URL.Host || scheme != req.URL.Scheme } // Fixup the request if needed. diff --git a/backend/s3/s3_test.go b/backend/s3/s3_test.go index d4d4fb84b..a10416dbb 100644 --- a/backend/s3/s3_test.go +++ b/backend/s3/s3_test.go @@ -115,6 +115,54 @@ func TestClientKeepsSecurityTokenOnSameHostRedirect(t *testing.T) { assert.NoError(t, resp.Body.Close()) } +func TestRedirectCrossesHost(t *testing.T) { + mustReq := func(method, url string) *http.Request { + req, err := http.NewRequest(method, url, nil) + require.NoError(t, err) + return req + } + for _, test := range []struct { + name string + via []string + req string + want bool + }{ + { + name: "SameHost", + via: []string{"https://bucket.example.com/"}, + req: "https://bucket.example.com/redirected", + want: false, + }, + { + name: "DifferentHost", + via: []string{"https://bucket.example.com/"}, + req: "https://evil.example.com/redirected", + want: true, + }, + { + name: "SchemeDowngradeSameHost", + via: []string{"https://bucket.example.com/"}, + req: "http://bucket.example.com/redirected", + want: true, + }, + { + name: "SchemeDowngradeMidChain", + via: []string{"https://bucket.example.com/", "http://bucket.example.com/middle"}, + req: "http://bucket.example.com/final", + want: true, + }, + } { + t.Run(test.name, func(t *testing.T) { + via := make([]*http.Request, len(test.via)) + for i, url := range test.via { + via[i] = mustReq(http.MethodGet, url) + } + got := s3RedirectCrossesHost(mustReq(http.MethodGet, test.req), via) + assert.Equal(t, test.want, got) + }) + } +} + func TestClientStopsAfterTenRedirects(t *testing.T) { _, _, client := SetupS3Test(t)