Visitar URL original
fix: prevent SSRF in google.List() pagination (#2332) · google/go-containerregistry@02649ea · GitHub
Skip to content

Commit 02649ea

Browse files
tufstrakatufstraka
authored
fix: prevent SSRF in google.List() pagination (#2332)
* fix: prevent SSRF in google.List() pagination Add URL validation to getNextPageURL() to ensure pagination Link headers point to the same registry host. This prevents SSRF attacks where a malicious registry could include a Link header pointing to internal services like cloud metadata endpoints (169.254.169.254). The fix validates that: 1. The pagination URL scheme matches the registry scheme 2. The pagination URL host matches the registry host This is consistent with SSRF protections added in PRs #2292, #2293, #2295 for other code paths. * fix: also prevent SSRF in remote.List() and remote.Catalog() pagination The same SSRF vulnerability that affected google.List() also exists in remote.List() and remote.Catalog(). A malicious registry can return a Link header pointing to internal services. This commit adds SSRF protection to the remote package's pagination handling, using the same validation approach. * fix: update remote list_test.go for getNextPageURL signature change Update test calls to getNextPageURL in pkg/v1/remote/list_test.go to pass the required name.Repository argument, matching the new function signature that includes SSRF validation. Also update makeResp helper to include a Request with URL for proper ResolveReference behavior, and adjust test assertions accordingly. * fix: treat empty Link header URL as no next page * fix: restore missing <> test case and add SSRF test to remote package --------- Co-authored-by: tufstraka <tufstraka@users.noreply.github.com>
1 parent 7204b40 commit 02649ea

5 files changed

Lines changed: 249 additions & 15 deletions

File tree

‎pkg/v1/google/list.go‎

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package google
1717
import (
1818
"context"
1919
"encoding/json"
20+
"errors"
2021
"fmt"
2122
"net/http"
2223
"net/url"
@@ -143,7 +144,7 @@ func (l *lister) list(repo name.Repository) (*Tags, error) {
143144
// This isn't GCR, just append the tags and keep paginating.
144145
tags.Tags = append(tags.Tags, parsed.Tags...)
145146

146-
uri, err = getNextPageURL(resp)
147+
uri, err = getNextPageURL(resp, repo)
147148
if err != nil {
148149
return nil, err
149150
}
@@ -159,8 +160,11 @@ func (l *lister) list(repo name.Repository) (*Tags, error) {
159160

160161
// getNextPageURL checks if there is a Link header in a http.Response which
161162
// contains a link to the next page. If yes it returns the url.URL of the next
162-
// page otherwise it returns nil.
163-
func getNextPageURL(resp *http.Response) (*url.URL, error) {
163+
// page otherwise it returns nil. It validates that the resolved URL points
164+
// to the same registry to prevent SSRF attacks where a malicious registry
165+
// could redirect pagination requests to internal services like cloud metadata
166+
// endpoints.
167+
func getNextPageURL(resp *http.Response, repo name.Repository) (*url.URL, error) {
164168
link := resp.Header.Get("Link")
165169
if link == "" {
166170
return nil, nil
@@ -175,6 +179,9 @@ func getNextPageURL(resp *http.Response) (*url.URL, error) {
175179
return nil, fmt.Errorf("failed to parse link header: missing '>' in: %s", link)
176180
}
177181
link = link[1:end]
182+
if link == "" {
183+
return nil, nil
184+
}
178185

179186
linkURL, err := url.Parse(link)
180187
if err != nil {
@@ -184,9 +191,33 @@ func getNextPageURL(resp *http.Response) (*url.URL, error) {
184191
return nil, nil
185192
}
186193
linkURL = resp.Request.URL.ResolveReference(linkURL)
194+
195+
// Validate that the pagination URL points to the same registry to prevent SSRF.
196+
if err := validatePaginationURL(linkURL, repo); err != nil {
197+
return nil, err
198+
}
199+
187200
return linkURL, nil
188201
}
189202

203+
// validatePaginationURL checks that a pagination URL is safe to follow.
204+
// It ensures the URL points to the same registry host to prevent SSRF attacks
205+
// where a malicious registry could include a Link header pointing to internal
206+
// services (e.g., cloud metadata endpoints at 169.254.169.254).
207+
func validatePaginationURL(u *url.URL, repo name.Repository) error {
208+
// Must use same scheme as the registry
209+
if u.Scheme != repo.Scheme() {
210+
return fmt.Errorf("pagination URL scheme %q does not match registry scheme %q", u.Scheme, repo.Scheme())
211+
}
212+
213+
// Must point to the same registry host
214+
if u.Host != repo.RegistryStr() {
215+
return errors.New("pagination URL host does not match registry host: potential SSRF attack")
216+
}
217+
218+
return nil
219+
}
220+
190221
type rawManifestInfo struct {
191222
Size string `json:"imageSizeBytes"`
192223
MediaType string `json:"mediaType"`

‎pkg/v1/google/list_test.go‎

Lines changed: 104 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,39 +301,139 @@ func makeResp(hdr string) *http.Response {
301301
Header: http.Header{
302302
"Link": []string{hdr},
303303
},
304+
Request: &http.Request{
305+
URL: &url.URL{
306+
Scheme: "https",
307+
Host: "example.com",
308+
},
309+
},
304310
}
305311
}
306312

307313
func TestGetNextPageURL(t *testing.T) {
314+
repo, err := name.NewRepository("example.com/myrepo")
315+
if err != nil {
316+
t.Fatalf("failed to create repository: %v", err)
317+
}
318+
308319
for _, hdr := range []string{
309320
"",
310321
"<",
311322
"><",
312323
"<>",
313324
fmt.Sprintf("<%c>", 0x7f), // makes url.Parse fail
314325
} {
315-
u, err := getNextPageURL(makeResp(hdr))
326+
u, err := getNextPageURL(makeResp(hdr), repo)
316327
if err == nil && u != nil {
317-
t.Errorf("Expected err, got %+v", u)
328+
t.Errorf("Expected err or nil URL for %q, got %+v", hdr, u)
318329
}
319330
}
320331

321332
good := &http.Response{
322333
Header: http.Header{
323-
"Link": []string{"<example.com>"},
334+
"Link": []string{"</v2/myrepo/tags/list?n=100>"},
324335
},
325336
Request: &http.Request{
326337
URL: &url.URL{
327338
Scheme: "https",
339+
Host: "example.com",
340+
Path: "/v2/myrepo/tags/list",
328341
},
329342
},
330343
}
331-
u, err := getNextPageURL(good)
344+
u, err := getNextPageURL(good, repo)
332345
if err != nil {
333346
t.Fatal(err)
334347
}
335348

336349
if u.Scheme != "https" {
337350
t.Errorf("expected scheme to match request, got %s", u.Scheme)
338351
}
352+
if u.Host != "example.com" {
353+
t.Errorf("expected host to match request, got %s", u.Host)
354+
}
355+
}
356+
357+
func TestValidatePaginationURL(t *testing.T) {
358+
repo, err := name.NewRepository("registry.example.com/myrepo")
359+
if err != nil {
360+
t.Fatalf("failed to create repository: %v", err)
361+
}
362+
363+
tests := []struct {
364+
name string
365+
url string
366+
wantErr bool
367+
}{
368+
{
369+
name: "same host - valid",
370+
url: "https://registry.example.com/v2/myrepo/tags/list?n=100&last=tag1",
371+
wantErr: false,
372+
},
373+
{
374+
name: "cloud metadata endpoint - SSRF attempt",
375+
url: "http://169.254.169.254/latest/meta-data/",
376+
wantErr: true,
377+
},
378+
{
379+
name: "localhost - SSRF attempt",
380+
url: "http://localhost:8080/internal",
381+
wantErr: true,
382+
},
383+
{
384+
name: "internal IP - SSRF attempt",
385+
url: "http://192.168.1.1/admin",
386+
wantErr: true,
387+
},
388+
{
389+
name: "different registry - SSRF attempt",
390+
url: "https://evil-registry.com/v2/myrepo/tags/list",
391+
wantErr: true,
392+
},
393+
{
394+
name: "scheme mismatch - potential downgrade",
395+
url: "http://registry.example.com/v2/myrepo/tags/list",
396+
wantErr: true,
397+
},
398+
}
399+
400+
for _, tc := range tests {
401+
t.Run(tc.name, func(t *testing.T) {
402+
u, err := url.Parse(tc.url)
403+
if err != nil {
404+
t.Fatalf("failed to parse URL: %v", err)
405+
}
406+
407+
err = validatePaginationURL(u, repo)
408+
if tc.wantErr && err == nil {
409+
t.Errorf("validatePaginationURL(%q) = nil, want error", tc.url)
410+
}
411+
if !tc.wantErr && err != nil {
412+
t.Errorf("validatePaginationURL(%q) = %v, want nil", tc.url, err)
413+
}
414+
})
415+
}
416+
}
417+
418+
func TestGetNextPageURL_SSRF(t *testing.T) {
419+
repo, _ := name.NewRepository("registry.example.com/myrepo")
420+
421+
// Malicious registry returns Link header pointing to cloud metadata
422+
malicious := &http.Response{
423+
Header: http.Header{
424+
"Link": []string{"<http://169.254.169.254/latest/meta-data/>;rel=\"next\""},
425+
},
426+
Request: &http.Request{
427+
URL: &url.URL{
428+
Scheme: "https",
429+
Host: "registry.example.com",
430+
Path: "/v2/myrepo/tags/list",
431+
},
432+
},
433+
}
434+
435+
_, err := getNextPageURL(malicious, repo)
436+
if err == nil {
437+
t.Error("getNextPageURL should reject Link header pointing to cloud metadata endpoint")
438+
}
339439
}

‎pkg/v1/remote/catalog.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -122,7 +122,7 @@ func (f *fetcher) catalogPage(ctx context.Context, reg name.Registry, next strin
122122
return nil, err
123123
}
124124

125-
uri, err := getNextPageURL(resp)
125+
uri, err := getNextPageURLForRegistry(resp, reg)
126126
if err != nil {
127127
return nil, err
128128
}

‎pkg/v1/remote/list.go‎

Lines changed: 67 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ package remote
1717
import (
1818
"context"
1919
"encoding/json"
20+
"errors"
2021
"fmt"
2122
"net/http"
2223
"net/url"
@@ -85,7 +86,7 @@ func (f *fetcher) listPage(ctx context.Context, repo name.Repository, next strin
8586
return nil, err
8687
}
8788

88-
uri, err := getNextPageURL(resp)
89+
uri, err := getNextPageURL(resp, repo)
8990
if err != nil {
9091
return nil, err
9192
}
@@ -99,8 +100,9 @@ func (f *fetcher) listPage(ctx context.Context, repo name.Repository, next strin
99100

100101
// getNextPageURL checks if there is a Link header in a http.Response which
101102
// contains a link to the next page. If yes it returns the url.URL of the next
102-
// page otherwise it returns nil.
103-
func getNextPageURL(resp *http.Response) (*url.URL, error) {
103+
// page otherwise it returns nil. It validates that the resolved URL points
104+
// to the same registry to prevent SSRF attacks.
105+
func getNextPageURL(resp *http.Response, repo name.Repository) (*url.URL, error) {
104106
link := resp.Header.Get("Link")
105107
if link == "" {
106108
return nil, nil
@@ -115,6 +117,9 @@ func getNextPageURL(resp *http.Response) (*url.URL, error) {
115117
return nil, fmt.Errorf("failed to parse link header: missing '>' in: %s", link)
116118
}
117119
link = link[1:end]
120+
if link == "" {
121+
return nil, nil
122+
}
118123

119124
linkURL, err := url.Parse(link)
120125
if err != nil {
@@ -124,9 +129,31 @@ func getNextPageURL(resp *http.Response) (*url.URL, error) {
124129
return nil, nil
125130
}
126131
linkURL = resp.Request.URL.ResolveReference(linkURL)
132+
133+
// Validate that the pagination URL points to the same registry to prevent SSRF.
134+
if err := validatePaginationURL(linkURL, repo); err != nil {
135+
return nil, err
136+
}
137+
127138
return linkURL, nil
128139
}
129140

141+
// validatePaginationURL checks that a pagination URL is safe to follow.
142+
func validatePaginationURL(u *url.URL, repo name.Repository) error {
143+
return validatePaginationURLHost(u, repo.Scheme(), repo.RegistryStr())
144+
}
145+
146+
// validatePaginationURLHost checks that a pagination URL is safe to follow.
147+
func validatePaginationURLHost(u *url.URL, scheme, host string) error {
148+
if u.Scheme != scheme {
149+
return fmt.Errorf("pagination URL scheme %q does not match registry scheme %q", u.Scheme, scheme)
150+
}
151+
if u.Host != host {
152+
return errors.New("pagination URL host does not match registry host: potential SSRF attack")
153+
}
154+
return nil
155+
}
156+
130157
type Lister struct {
131158
f *fetcher
132159
repo name.Repository
@@ -150,3 +177,40 @@ func (l *Lister) Next(ctx context.Context) (*Tags, error) {
150177
func (l *Lister) HasNext() bool {
151178
return l.page != nil && (!l.needMore || l.page.Next != "")
152179
}
180+
181+
// getNextPageURLForRegistry is like getNextPageURL but for name.Registry.
182+
func getNextPageURLForRegistry(resp *http.Response, reg name.Registry) (*url.URL, error) {
183+
link := resp.Header.Get("Link")
184+
if link == "" {
185+
return nil, nil
186+
}
187+
188+
if link[0] != '<' {
189+
return nil, fmt.Errorf("failed to parse link header: missing '<' in: %s", link)
190+
}
191+
192+
end := strings.Index(link, ">")
193+
if end == -1 {
194+
return nil, fmt.Errorf("failed to parse link header: missing '>' in: %s", link)
195+
}
196+
link = link[1:end]
197+
if link == "" {
198+
return nil, nil
199+
}
200+
201+
linkURL, err := url.Parse(link)
202+
if err != nil {
203+
return nil, err
204+
}
205+
if resp.Request == nil || resp.Request.URL == nil {
206+
return nil, nil
207+
}
208+
linkURL = resp.Request.URL.ResolveReference(linkURL)
209+
210+
// Validate that the pagination URL points to the same registry to prevent SSRF.
211+
if err := validatePaginationURLHost(linkURL, reg.Scheme(), reg.RegistryStr()); err != nil {
212+
return nil, err
213+
}
214+
215+
return linkURL, nil
216+
}

0 commit comments

Comments
 (0)