fix(security): pin registry Location headers to original origin
Reject cross-origin Location redirects in the native registry push client. Previously, resolveLocation() blindly followed any Location header, allowing a malicious registry to redirect blob uploads to an attacker-controlled host while credentials were still attached.
- commit
- aa2bfce5557600d4537cbd0e513c9a7c3407c0f5
- author
- brice · · 2026-03-14 22:09 UTC
- parent
- 103b727
- signature
- unsigned
2 files · +35 -1
M pkg/registrypush/push.go +7 -1
| @@ -502,7 +502,13 @@ func resolveLocation(baseURL, location string) (string, error) { | |||
| 502 | 502 | return "", err | |
| 503 | 503 | } | |
| 504 | 504 | ||
| 505 | - | return base.ResolveReference(ref).String(), nil | |
| 505 | + | resolved := base.ResolveReference(ref) | |
| 506 | + | ||
| 507 | + | if resolved.Scheme != base.Scheme || resolved.Host != base.Host { | |
| 508 | + | return "", fmt.Errorf("registry returned cross-origin Location %q (expected %s://%s)", location, base.Scheme, base.Host) | |
| 509 | + | } | |
| 510 | + | ||
| 511 | + | return resolved.String(), nil | |
| 506 | 512 | } | |
| 507 | 513 | ||
| 508 | 514 | func chunkCount(size, chunkSize int64) int64 { | |
| @@ -502,7 +502,13 @@ func resolveLocation(baseURL, location string) (string, error) { | |||||
| 502 | return "", err | 502 | return "", err | ||
| 503 | } | 503 | } | ||
| 504 | 504 | ||||
| 505 | - | return base.ResolveReference(ref).String(), nil | 505 | + | resolved := base.ResolveReference(ref) |
| 506 | + | ||||
| 507 | + | if resolved.Scheme != base.Scheme || resolved.Host != base.Host { | |||
| 508 | + | return "", fmt.Errorf("registry returned cross-origin Location %q (expected %s://%s)", location, base.Scheme, base.Host) | |||
| 509 | + | } | |||
| 510 | + | ||||
| 511 | + | return resolved.String(), nil | |||
| 506 | } | 512 | } | ||
| 507 | 513 | ||||
| 508 | func chunkCount(size, chunkSize int64) int64 { | 514 | func chunkCount(size, chunkSize int64) int64 { | ||
M pkg/registrypush/push_test.go +28 -0
| @@ -256,6 +256,34 @@ func TestPusher_WithInsecureTLS_AllowsSelfSignedCertificate(t *testing.T) { | |||
| 256 | 256 | require.NoError(t, err) | |
| 257 | 257 | } | |
| 258 | 258 | ||
| 259 | + | func TestUploadBlob_RejectsCrossOriginRedirect(t *testing.T) { | |
| 260 | + | attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 261 | + | t.Errorf("request leaked to attacker: %s %s (Authorization: %s)", r.Method, r.URL.Path, r.Header.Get("Authorization")) | |
| 262 | + | w.WriteHeader(http.StatusOK) | |
| 263 | + | })) | |
| 264 | + | defer attacker.Close() | |
| 265 | + | ||
| 266 | + | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |
| 267 | + | switch { | |
| 268 | + | case r.Method == http.MethodHead: | |
| 269 | + | w.WriteHeader(http.StatusNotFound) | |
| 270 | + | case r.Method == http.MethodPost: | |
| 271 | + | w.Header().Set("Location", attacker.URL+"/v2/repo/blobs/uploads/evil") | |
| 272 | + | w.WriteHeader(http.StatusAccepted) | |
| 273 | + | default: | |
| 274 | + | w.WriteHeader(http.StatusInternalServerError) | |
| 275 | + | } | |
| 276 | + | })) | |
| 277 | + | defer registry.Close() | |
| 278 | + | ||
| 279 | + | p := registrypush.New(registrypush.WithChunkSize(1024)) | |
| 280 | + | ctx := context.Background() | |
| 281 | + | err := p.UploadBlob(ctx, registry.URL, "test/repo", "sha256:abc", 5, strings.NewReader("hello"), "Basic secret") | |
| 282 | + | ||
| 283 | + | require.Error(t, err) | |
| 284 | + | assert.Contains(t, err.Error(), "cross-origin") | |
| 285 | + | } | |
| 286 | + | ||
| 259 | 287 | type fakeBlobUploadRegistry struct { | |
| 260 | 288 | t *testing.T | |
| 261 | 289 | expectedAuth string | |
| @@ -256,6 +256,34 @@ func TestPusher_WithInsecureTLS_AllowsSelfSignedCertificate(t *testing.T) { | |||||
| 256 | require.NoError(t, err) | 256 | require.NoError(t, err) | ||
| 257 | } | 257 | } | ||
| 258 | 258 | ||||
| 259 | + | func TestUploadBlob_RejectsCrossOriginRedirect(t *testing.T) { | |||
| 260 | + | attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 261 | + | t.Errorf("request leaked to attacker: %s %s (Authorization: %s)", r.Method, r.URL.Path, r.Header.Get("Authorization")) | |||
| 262 | + | w.WriteHeader(http.StatusOK) | |||
| 263 | + | })) | |||
| 264 | + | defer attacker.Close() | |||
| 265 | + | ||||
| 266 | + | registry := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { | |||
| 267 | + | switch { | |||
| 268 | + | case r.Method == http.MethodHead: | |||
| 269 | + | w.WriteHeader(http.StatusNotFound) | |||
| 270 | + | case r.Method == http.MethodPost: | |||
| 271 | + | w.Header().Set("Location", attacker.URL+"/v2/repo/blobs/uploads/evil") | |||
| 272 | + | w.WriteHeader(http.StatusAccepted) | |||
| 273 | + | default: | |||
| 274 | + | w.WriteHeader(http.StatusInternalServerError) | |||
| 275 | + | } | |||
| 276 | + | })) | |||
| 277 | + | defer registry.Close() | |||
| 278 | + | ||||
| 279 | + | p := registrypush.New(registrypush.WithChunkSize(1024)) | |||
| 280 | + | ctx := context.Background() | |||
| 281 | + | err := p.UploadBlob(ctx, registry.URL, "test/repo", "sha256:abc", 5, strings.NewReader("hello"), "Basic secret") | |||
| 282 | + | ||||
| 283 | + | require.Error(t, err) | |||
| 284 | + | assert.Contains(t, err.Error(), "cross-origin") | |||
| 285 | + | } | |||
| 286 | + | ||||
| 259 | type fakeBlobUploadRegistry struct { | 287 | type fakeBlobUploadRegistry struct { | ||
| 260 | t *testing.T | 288 | t *testing.T | ||
| 261 | expectedAuth string | 289 | expectedAuth string | ||