fix: prevent SSRF, stack overflow on circular $ref, and path traversal in local file reads#27
Open
evilgensec wants to merge 3 commits into
Open
Conversation
A malicious OpenAPI spec (or Google Discovery document) can embed $ref values pointing at arbitrary URLs, including private/link-local IP ranges such as the cloud instance-metadata endpoint 169.254.169.254. Prior to this change, fetchFile called http.Get without any URL validation, so processing an untrusted spec in a cloud environment would silently exfiltrate IAM credentials or service-account tokens to the attacker-controlled $ref host. Add validateRemoteURL, which rejects: - non-HTTP(S) schemes (file://, ftp://, etc.) - IP literals that are loopback, link-local, private (RFC 1918), or unspecified (0.0.0.0) The check mirrors the approach used by go-containerregistry's validateRealmURL to block SSRF via WWW-Authenticate realm injection. DNS-based SSRF is explicitly out of scope (consistent with that precedent); network-level controls should be applied where needed. Add compiler/reader_test.go with TestValidateRemoteURL (12 table cases) and TestFetchFileSSRF (end-to-end rejection of loopback and the AWS/GCP metadata endpoint).
The previous fix validated the initial URL before calling http.Get, but http.Get follows redirects by default. A malicious $ref can point at a public host that the caller controls; that host issues a 302 to http://169.254.169.254/credentials (AWS/GCP instance-metadata), and the Go HTTP client transparently fetches the private endpoint. Add a CheckRedirect hook to the one-shot http.Client used in fetchFile so that every redirect destination is passed through validateRemoteURL before the hop is followed. A redirect to a loopback or link-local address now returns an error instead of leaking instance credentials. New tests: - TestFetchFileSSRFViaRedirect: two httptest servers (attacker + victim); confirms the redirect is stopped with a "private or link-local" error. - TestFetchFileValidPublicURLAllowed: regression guard — public URLs still produce the expected validation path.
…cal file reads Two security fixes: 1. openapiv2/OpenAPIv2.go, openapiv3/OpenAPIv3.go: ResolveReferences recursively resolved $ref values with no cycle detection. A circular reference (A.$ref -> B, B.$ref -> A) causes unbounded recursion and a stack overflow crash. Fix: convert the tail recursion to an iterative loop with a visited-set; return an error if a cycle is detected. 2. compiler/reader.go: ReadInfoForRef constructed local file paths as basedir + parts[0] without sanitization, allowing relative paths (../../etc/passwd) or absolute URI paths (/etc/passwd) in $ref values to read arbitrary local files. Fix: use filepath.Join and verify the resolved path has basedir as a prefix; reject $ref values with non-http/https schemes.
evilgensec
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Four security fixes for OpenAPI spec parsing in gnostic-models:
1. SSRF via external
$refURL (commits 7303496, 0eb4686)fetchFilefetched remote$refURLs with no validation — any URL includinghttp://169.254.169.254/latest/meta-data/could be used to exfiltrate cloud instance metadata. Additionally, theCheckRedirecthook only validated initial URLs; a redirect to a private endpoint bypassed the check.Fix: Validate remote URLs against private/loopback/link-local ranges; block redirects to private hosts.
2. Stack overflow via circular
$ref(OpenAPI v2 + v3) — commit 1c50076Schema.ResolveReferences(openapiv2) andReference.ResolveReferences(openapiv3) resolved$refvalues with unconditional tail recursion and no cycle detection. A circular reference (A.$ref → B,B.$ref → A) causes unbounded recursion and a goroutine stack overflow, crashing the process.Fix: Convert tail recursion to an iterative loop with a visited-set; return an error when a cycle is detected.
3. Arbitrary local file read via
$refpath traversal — commit 1c50076ReadInfoForRefconstructed local paths asbasedir + parts[0]without sanitization, allowing../../etc/passwdto escape the base directory. Additionally,url.ParseRequestURI("/etc/passwd")treats absolute paths as valid URIs, letting/etc/passwdbypass URL filtering and be read as a local file.Fix: Use
filepath.Join+ prefix check to contain local refs; reject non-http/https schemes.Tests added
openapiv2/circular_ref_test.go,openapiv3/circular_ref_test.go: circular $ref returns error; non-circular resolves.compiler/reader_test.go: path traversal and disallowed schemes rejected; same-directory refs succeed.