mirror of
https://github.com/rclone/rclone.git
synced 2026-07-13 09:12:31 -04:00
s3: strip STS security token on same-host HTTPS->HTTP redirect GHSA-cf44-9pgv-m4xc
The CheckRedirect policy strips the X-Amz-Security-Token header when a redirect chain crosses a host, but it only compared the host and ignored the scheme. A redirect that kept the same host:port but downgraded https:// to http:// was treated as the same host, so the STS session token was re-sent over a plaintext connection where it could be observed. Fixes GHSA-cf44-9pgv-m4xc
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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)
|
||||
|
||||
|
||||
Reference in New Issue
Block a user