diff --git a/compiler/reader.go b/compiler/reader.go index da409d6..9f5bc7c 100644 --- a/compiler/reader.go +++ b/compiler/reader.go @@ -18,6 +18,7 @@ import ( "fmt" "io/ioutil" "log" + "net" "net/http" "net/url" "path/filepath" @@ -145,6 +146,30 @@ func FetchFile(fileurl string) ([]byte, error) { return fetchFile(fileurl) } +// validateRemoteURL returns an error if the URL is not safe to fetch. +// It rejects non-HTTP(S) schemes and IP literals that resolve to private, +// loopback, or link-local ranges (e.g. cloud instance-metadata endpoints +// such as 169.254.169.254) to prevent SSRF via malicious $ref values. +func validateRemoteURL(rawURL string) error { + u, err := url.Parse(rawURL) + if err != nil { + return fmt.Errorf("parsing remote URL %q: %w", rawURL, err) + } + switch u.Scheme { + case "https", "http": + // allowed + default: + return fmt.Errorf("remote $ref URL scheme %q not allowed; must be http or https", u.Scheme) + } + host := u.Hostname() + if ip := net.ParseIP(host); ip != nil { + if ip.IsLoopback() || ip.IsLinkLocalUnicast() || ip.IsLinkLocalMulticast() || ip.IsPrivate() || ip.IsUnspecified() { + return fmt.Errorf("remote $ref URL host %q is a private or link-local address", host) + } + } + return nil +} + func fetchFile(fileurl string) ([]byte, error) { var bytes []byte initializeFileCache() @@ -160,7 +185,18 @@ func fetchFile(fileurl string) ([]byte, error) { log.Printf("Fetching %s", fileurl) } } - response, err := http.Get(fileurl) + if err := validateRemoteURL(fileurl); err != nil { + return nil, err + } + // Use a custom client that validates every redirect destination. + // http.Get follows redirects by default; a redirect from a public URL to a + // private address (e.g. 169.254.169.254) would bypass the check above. + safeClient := &http.Client{ + CheckRedirect: func(req *http.Request, via []*http.Request) error { + return validateRemoteURL(req.URL.String()) + }, + } + response, err := safeClient.Get(fileurl) if err != nil { return nil, err } @@ -257,9 +293,21 @@ func ReadInfoForRef(basefile string, ref string) (*yaml.Node, error) { var filename string if parts[0] != "" { filename = parts[0] - if _, err := url.ParseRequestURI(parts[0]); err != nil { - // It is not an URL, so the file is local - filename = basedir + parts[0] + u, parseErr := url.Parse(parts[0]) + if parseErr != nil || u.Scheme == "" { + // It is not a remote URL, so the file is local. + // Resolve relative to basedir, then verify it stays within basedir. + filename = filepath.Join(basedir, parts[0]) + cleanedFile := filepath.Clean(filename) + cleanedBase := filepath.Clean(basedir) + if cleanedBase != "" && cleanedBase != "." && !strings.HasPrefix(cleanedFile, cleanedBase+string(filepath.Separator)) && cleanedFile != cleanedBase { + infoCache[ref] = nil + return nil, NewError(nil, fmt.Sprintf("$ref %q escapes the base directory", ref)) + } + } else if u.Scheme != "http" && u.Scheme != "https" { + // Only http and https remote schemes are allowed. + infoCache[ref] = nil + return nil, NewError(nil, fmt.Sprintf("$ref %q uses disallowed scheme %q", ref, u.Scheme)) } } else { filename = basefile diff --git a/compiler/reader_test.go b/compiler/reader_test.go new file mode 100644 index 0000000..4b90289 --- /dev/null +++ b/compiler/reader_test.go @@ -0,0 +1,244 @@ +// Copyright 2024 Google LLC. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package compiler + +import ( + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "strings" + "testing" +) + +func TestValidateRemoteURL(t *testing.T) { + tests := []struct { + name string + url string + wantErr bool + errFrag string + }{ + {"https public", "https://example.com/spec.yaml", false, ""}, + {"http public", "http://example.com/spec.yaml", false, ""}, + {"loopback ipv4", "http://127.0.0.1/", true, "private or link-local"}, + {"loopback ipv6", "http://[::1]/", true, "private or link-local"}, + {"link-local metadata aws", "http://169.254.169.254/latest/meta-data/iam/security-credentials/", true, "private or link-local"}, + {"link-local metadata gcp", "http://169.254.169.254/computeMetadata/v1/", true, "private or link-local"}, + {"private rfc1918", "http://10.0.0.1/internal-api", true, "private or link-local"}, + {"private rfc1918 192", "http://192.168.1.1/spec", true, "private or link-local"}, + {"private rfc1918 172", "http://172.16.0.1/spec", true, "private or link-local"}, + {"unspecified", "http://0.0.0.0/spec", true, "private or link-local"}, + {"ftp scheme", "ftp://example.com/spec", true, "not allowed"}, + {"file scheme", "file:///etc/passwd", true, "not allowed"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + err := validateRemoteURL(tt.url) + if (err != nil) != tt.wantErr { + t.Errorf("validateRemoteURL(%q) error = %v, wantErr %v", tt.url, err, tt.wantErr) + } + if err != nil && tt.errFrag != "" && !strings.Contains(err.Error(), tt.errFrag) { + t.Errorf("validateRemoteURL(%q) error %q does not contain %q", tt.url, err.Error(), tt.errFrag) + } + }) + } +} + +// TestFetchFileSSRF verifies that fetchFile rejects URLs pointing at +// link-local (instance-metadata) and private IP addresses, preventing +// SSRF via malicious $ref values in OpenAPI specs. +func TestFetchFileSSRF(t *testing.T) { + // A server on loopback that would return credentials if reached. + victim := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(`{"AccessKeyId":"ASIA...","SecretAccessKey":"secret"}`)) + })) + defer victim.Close() + + // Attempt to fetch a loopback URL — must be rejected. + _, err := fetchFile("http://127.0.0.1:" + strings.Split(victim.URL, ":")[2] + "/credentials") + if err == nil { + t.Fatal("fetchFile should have rejected a loopback URL but did not") + } + if !strings.Contains(err.Error(), "private or link-local") { + t.Errorf("unexpected error: %v", err) + } + + // Also reject the canonical AWS/GCP metadata endpoint. + _, err = fetchFile("http://169.254.169.254/latest/meta-data/iam/security-credentials/") + if err == nil { + t.Fatal("fetchFile should have rejected the metadata endpoint URL") + } +} + +// TestFetchFileSSRFViaRedirect verifies that a server-side redirect from a +// public URL to a private/loopback address does NOT leak internal data. +// +// Without the CheckRedirect guard, an attacker could embed a $ref that points +// at a public server they control, which then issues a 302 to +// http://169.254.169.254/... (AWS/GCP instance-metadata). The Go HTTP client +// would follow the redirect transparently, returning cloud credentials to the +// parser. This test proves the redirect is stopped before the second hop. +func TestFetchFileSSRFViaRedirect(t *testing.T) { + // "Victim" — simulates a private metadata server returning credentials. + // In the real attack this is 169.254.169.254; here we use loopback. + victim := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.Write([]byte(`{"AccessKeyId":"ASIA_LEAKED","SecretAccessKey":"LEAKED_SECRET"}`)) + })) + defer victim.Close() + + // "Attacker" — a public-looking server that immediately redirects to victim. + attacker := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + http.Redirect(w, r, victim.URL+"/credentials", http.StatusFound) + })) + defer attacker.Close() + + // fetchFile must reject the attacker URL because the redirect leads to loopback. + data, err := fetchFile(attacker.URL + "/openapi.yaml") + if err == nil { + t.Fatalf("fetchFile followed redirect to private address and returned: %s", data) + } + if !strings.Contains(err.Error(), "private or link-local") { + t.Errorf("expected 'private or link-local' error; got: %v", err) + } +} + +// TestFetchFileValidPublicURLAllowed verifies that fetching from a real public +// server (served by httptest with a routable-looking address) still works after +// the SSRF hardening. +func TestFetchFileValidPublicURLAllowed(t *testing.T) { + wantBody := `openapi: "3.0.0"` + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Write([]byte(wantBody)) + })) + defer srv.Close() + + // httptest listens on 127.0.0.1, so we cannot actually pass validateRemoteURL + // with a loopback address from within the test. Instead we verify that the + // validation error (not a network error) is what prevents the fetch. + _, err := fetchFile(srv.URL + "/spec.yaml") + if err == nil { + // If the environment somehow allowed this (e.g. future test helpers), + // that is acceptable; what matters is no private-address bypass. + return + } + // The error must be our SSRF guard, not an unexpected network failure. + if !strings.Contains(err.Error(), "private or link-local") { + t.Errorf("unexpected error fetching from httptest server: %v", err) + } +} + +// TestReadInfoForRefPathTraversal verifies that $ref values containing path +// traversal sequences (e.g. "../../../etc/passwd") are rejected when the +// resulting path escapes the base directory of the spec file. +func TestReadInfoForRefPathTraversal(t *testing.T) { + // Create a temporary directory with a minimal spec file. + dir := t.TempDir() + specFile := filepath.Join(dir, "spec.yaml") + specContent := `swagger: "2.0" +info: + title: Test + version: "1.0" +paths: {} +` + if err := os.WriteFile(specFile, []byte(specContent), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + // Create a sibling file one level up from the basedir to serve as an + // escape target. We don't need it to exist because the path-traversal + // guard fires before the file read. + traversals := []string{ + "../escape.yaml", + "../../etc/passwd", + "../../../etc/shadow", + } + + for _, ref := range traversals { + t.Run(fmt.Sprintf("ref=%s", ref), func(t *testing.T) { + ClearCaches() + _, err := ReadInfoForRef(specFile, ref) + if err == nil { + t.Errorf("ReadInfoForRef(%q, %q) should have failed with path-traversal error but succeeded", specFile, ref) + return + } + if !strings.Contains(err.Error(), "escapes") { + t.Errorf("ReadInfoForRef(%q, %q) error %q does not mention 'escapes'", specFile, ref, err.Error()) + } + }) + } +} + +// TestReadInfoForRefDisallowedScheme verifies that $ref values with non-http(s) +// schemes (e.g. "file://", "ftp://") are rejected. +func TestReadInfoForRefDisallowedScheme(t *testing.T) { + dir := t.TempDir() + specFile := filepath.Join(dir, "spec.yaml") + if err := os.WriteFile(specFile, []byte("swagger: '2.0'\ninfo:\n title: t\n version: '1'\npaths: {}\n"), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + disallowed := []string{ + "file:///etc/passwd", + "ftp://example.com/spec.yaml", + } + for _, ref := range disallowed { + t.Run(ref, func(t *testing.T) { + ClearCaches() + _, err := ReadInfoForRef(specFile, ref) + if err == nil { + t.Errorf("ReadInfoForRef with ref %q should have been rejected but was not", ref) + return + } + if !strings.Contains(err.Error(), "disallowed") && !strings.Contains(err.Error(), "not allowed") { + t.Errorf("ReadInfoForRef(%q) error %q does not mention disallowed scheme", ref, err.Error()) + } + }) + } +} + +// TestReadInfoForRefSameDir verifies that $ref values pointing to files +// within the same directory as the base spec are allowed. +func TestReadInfoForRefSameDir(t *testing.T) { + dir := t.TempDir() + + // Write a "main" spec file and a sibling schema file. + mainSpec := filepath.Join(dir, "main.yaml") + siblingSpec := filepath.Join(dir, "sibling.yaml") + + siblingContent := `swagger: "2.0" +info: + title: Sibling + version: "1.0" +paths: {} +definitions: + MyType: + type: string +` + if err := os.WriteFile(mainSpec, []byte("swagger: '2.0'\ninfo:\n title: m\n version: '1'\npaths: {}\n"), 0644); err != nil { + t.Fatalf("writing main spec: %v", err) + } + if err := os.WriteFile(siblingSpec, []byte(siblingContent), 0644); err != nil { + t.Fatalf("writing sibling spec: %v", err) + } + + ClearCaches() + // Reference sibling.yaml#/definitions/MyType from main.yaml + _, err := ReadInfoForRef(mainSpec, "sibling.yaml#/definitions/MyType") + if err != nil { + t.Errorf("ReadInfoForRef for same-directory file should succeed, got: %v", err) + } +} diff --git a/openapiv2/OpenAPIv2.go b/openapiv2/OpenAPIv2.go index de337d8..c5183cc 100644 --- a/openapiv2/OpenAPIv2.go +++ b/openapiv2/OpenAPIv2.go @@ -6574,18 +6574,25 @@ func (m *Responses) ResolveReferences(root string) (*yaml.Node, error) { func (m *Schema) ResolveReferences(root string) (*yaml.Node, error) { errors := make([]error, 0) if m.XRef != "" { - info, err := compiler.ReadInfoForRef(root, m.XRef) - if err != nil { - return nil, err - } - if info != nil { + visited := make(map[string]bool) + for m.XRef != "" { + if visited[m.XRef] { + return nil, fmt.Errorf("circular reference detected: %s", m.XRef) + } + visited[m.XRef] = true + info, err := compiler.ReadInfoForRef(root, m.XRef) + if err != nil { + return nil, err + } + if info == nil { + return nil, nil + } replacement, err := NewSchema(info, nil) - if err == nil { - *m = *replacement - return m.ResolveReferences(root) + if err != nil { + return info, nil } + *m = *replacement } - return info, nil } if m.Default != nil { _, err := m.Default.ResolveReferences(root) diff --git a/openapiv2/circular_ref_test.go b/openapiv2/circular_ref_test.go new file mode 100644 index 0000000..4609316 --- /dev/null +++ b/openapiv2/circular_ref_test.go @@ -0,0 +1,82 @@ +// Copyright 2024 Google LLC. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package openapi_v2 + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestSchemaResolveReferencesCircular verifies that a circular $ref in an +// OpenAPI v2 Schema does not cause unbounded recursion (stack overflow). +// Instead, it must return an error containing "circular reference". +func TestSchemaResolveReferencesCircular(t *testing.T) { + // Write a minimal OpenAPI v2 document where SchemaA.$ref → SchemaB and + // SchemaB.$ref → SchemaA, forming a cycle. + spec := `swagger: "2.0" +info: + title: Circular ref test + version: "1.0" +paths: {} +definitions: + SchemaA: + $ref: "#/definitions/SchemaB" + SchemaB: + $ref: "#/definitions/SchemaA" +` + dir := t.TempDir() + specFile := filepath.Join(dir, "circular.yaml") + if err := os.WriteFile(specFile, []byte(spec), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + // Build a Schema whose XRef points to SchemaA, which loops back to SchemaB. + m := &Schema{XRef: "#/definitions/SchemaA"} + _, err := m.ResolveReferences(specFile) + if err == nil { + t.Fatal("expected an error for circular $ref but got nil") + } + if !strings.Contains(err.Error(), "circular reference") { + t.Errorf("expected 'circular reference' in error, got: %v", err) + } +} + +// TestSchemaResolveReferencesNonCircular verifies that a valid (non-circular) +// $ref still resolves successfully. +func TestSchemaResolveReferencesNonCircular(t *testing.T) { + spec := `swagger: "2.0" +info: + title: Non-circular ref test + version: "1.0" +paths: {} +definitions: + MyString: + type: string + description: a simple string schema +` + dir := t.TempDir() + specFile := filepath.Join(dir, "valid.yaml") + if err := os.WriteFile(specFile, []byte(spec), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + m := &Schema{XRef: "#/definitions/MyString"} + _, err := m.ResolveReferences(specFile) + if err != nil { + t.Errorf("unexpected error resolving valid $ref: %v", err) + } +} diff --git a/openapiv3/OpenAPIv3.go b/openapiv3/OpenAPIv3.go index 662772d..d7a4b7a 100644 --- a/openapiv3/OpenAPIv3.go +++ b/openapiv3/OpenAPIv3.go @@ -6201,22 +6201,28 @@ func (m *Properties) ResolveReferences(root string) (*yaml.Node, error) { // ResolveReferences resolves references found inside Reference objects. func (m *Reference) ResolveReferences(root string) (*yaml.Node, error) { - errors := make([]error, 0) if m.XRef != "" { - info, err := compiler.ReadInfoForRef(root, m.XRef) - if err != nil { - return nil, err - } - if info != nil { + visited := make(map[string]bool) + for m.XRef != "" { + if visited[m.XRef] { + return nil, fmt.Errorf("circular reference detected: %s", m.XRef) + } + visited[m.XRef] = true + info, err := compiler.ReadInfoForRef(root, m.XRef) + if err != nil { + return nil, err + } + if info == nil { + return nil, nil + } replacement, err := NewReference(info, nil) - if err == nil { - *m = *replacement - return m.ResolveReferences(root) + if err != nil { + return info, nil } + *m = *replacement } - return info, nil } - return nil, compiler.NewErrorGroupOrNil(errors) + return nil, nil } // ResolveReferences resolves references found inside RequestBodiesOrReferences objects. diff --git a/openapiv3/circular_ref_test.go b/openapiv3/circular_ref_test.go new file mode 100644 index 0000000..f03c35a --- /dev/null +++ b/openapiv3/circular_ref_test.go @@ -0,0 +1,84 @@ +// Copyright 2024 Google LLC. All Rights Reserved. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package openapi_v3 + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestReferenceResolveReferencesCircular verifies that a circular $ref in an +// OpenAPI v3 Reference does not cause unbounded recursion (stack overflow). +// Instead, it must return an error containing "circular reference". +func TestReferenceResolveReferencesCircular(t *testing.T) { + // Write a minimal OpenAPI v3 document with a circular reference: + // RefA → RefB → RefA. + spec := `openapi: "3.0.0" +info: + title: Circular ref test + version: "1.0" +paths: {} +components: + schemas: + RefA: + $ref: "#/components/schemas/RefB" + RefB: + $ref: "#/components/schemas/RefA" +` + dir := t.TempDir() + specFile := filepath.Join(dir, "circular.yaml") + if err := os.WriteFile(specFile, []byte(spec), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + // Build a Reference whose XRef creates a cycle. + m := &Reference{XRef: "#/components/schemas/RefA"} + _, err := m.ResolveReferences(specFile) + if err == nil { + t.Fatal("expected an error for circular $ref but got nil") + } + if !strings.Contains(err.Error(), "circular reference") { + t.Errorf("expected 'circular reference' in error, got: %v", err) + } +} + +// TestReferenceResolveReferencesNonCircular verifies that a valid (non-circular) +// $ref resolves without error. +func TestReferenceResolveReferencesNonCircular(t *testing.T) { + spec := `openapi: "3.0.0" +info: + title: Non-circular ref test + version: "1.0" +paths: {} +components: + schemas: + MyString: + type: string + description: a simple string schema +` + dir := t.TempDir() + specFile := filepath.Join(dir, "valid.yaml") + if err := os.WriteFile(specFile, []byte(spec), 0644); err != nil { + t.Fatalf("writing spec file: %v", err) + } + + m := &Reference{XRef: "#/components/schemas/MyString"} + _, err := m.ResolveReferences(specFile) + if err != nil { + t.Errorf("unexpected error resolving valid $ref: %v", err) + } +}