compiler: validate $ref URLs in fetchFile to prevent SSRF#26
Open
evilgensec wants to merge 1 commit intogoogle:mainfrom
Open
compiler: validate $ref URLs in fetchFile to prevent SSRF#26evilgensec wants to merge 1 commit intogoogle:mainfrom
evilgensec wants to merge 1 commit intogoogle:mainfrom
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
fetchFile() previously called http.Get() with any URL derived from a $ref value in a user-supplied OpenAPI spec, with no validation of the scheme or destination host. This allowed an attacker to craft a spec that caused gnostic to make HTTP requests to internal network addresses, cloud instance metadata endpoints (e.g. 169.254.169.254), or arbitrary non-HTTP schemes. Add validateFetchURL() which enforces: - Scheme allowlist: only "http" and "https" are permitted. - Private/reserved IP blocklist: requests to RFC 1918 addresses, loopback (127.0.0.0/8), link-local (169.254.0.0/16 including cloud IMDS), RFC 6598 shared space, and IPv6 equivalents are rejected. fetchFile() now returns an error before issuing the HTTP request if the URL fails either check.
e47a8b3 to
be20f9d
Compare
Author
|
Please note that the vulnerability was reported through Google VRP and they redirected the issue to be forwarded here. Fixes SSRF vulnerability reported via Google VRP (Issue 495622065). @timburks @precision @rspier @morphar |
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
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
fetchFile()incompiler/reader.gocalledhttp.Get()with any URL derived from a$refvalue in a user-supplied OpenAPI specification, with no validation of the URL scheme or destination host. An attacker who can supply a crafted spec to a gnostic-based CI pipeline or tool can cause gnostic to make HTTP requests to:http://169.254.169.254/latest/meta-data/iam/security-credentials/)Root Cause
readBytesForFile()detects whether a$refvalue is a URL by checkingurl.Parse().Scheme != ""and, if true, passes the raw string directly tofetchFile().fetchFile()then callshttp.Get(fileurl)with no further validation.Fix
This commit adds
validateFetchURL()which is called insidefetchFile()before the HTTP request is issued. It enforces:httpandhttpsare permitted; any other scheme (e.g.file://,ftp://) is rejected with an error.10.0.0.0/8,172.16.0.0/12,192.168.0.0/16(RFC 1918)127.0.0.0/8(loopback)169.254.0.0/16(link-local / cloud IMDS)100.64.0.0/10(shared address space, RFC 6598)::1/128,fc00::/7,fe80::/10)Example — before patch
Example — after patch
Notes
readBytesForFilehandles the non-URL path separately and is unaffected).Fixes SSRF vulnerability reported via Google VRP (Issue 495622065).