Yet another SSRF query for Go

[npesaresi]

Query

Relevant PR: github/codeql-go#573

Report

This query finds SSRFs (Server Side Request Forgeries). This is intended to be an alternative to the current Request Forgery query.

About SSRF

A web server is vulnerable to SSRF when it receives some user input, uses it to form a URL and then sends a request to this URL. If the user input isn’t properly checked, an attacker could manipulate the URL and our web server will send the request elsewhere.
This request will be done in our app’s name. An attacker can use this to bypass a firewall, since our app will be trusted in the internal network, and read confidential information or execute unauthorized actions.

Why a new query?

We use codeql queries at Mercado Libre to block pull requests. We need them to be very precise.
We recommend input validation, not URL escaping, when fixing a SSRF. Some NGINx based routing tend to decode before routing, making our URL escape useless. But input validation always works.
So we adapted the original query to this blocking context and to accept different forms of input validation.

What's different?

The original query just looks for SSRFs that allow an attacker to change the domain in the URL. By this logic, this case http.Get("http://example.com/" + tainted) should be safe.
This doesn’t work for us. Many companies have an internal network where different apps are mapped to different paths sharing the same domain. So, being able to manipulate the path part of the URL would let an attacker navigate to all these apps, even if they can’t change the domain.
We changed the HostnameSanitizer for a PathSanitizer. Our query flags http.Get("http://example.com/" + tainted) as SSRF but http.Get("http://example.com/something?key=" + tainted) is considered safe, since we can’t manipulate the path nor the domain from the querystring.

We removed the RedirectCheckBarrierGuardAsBarrierGuard, we had too many false negatives. A function isn’t safe just because it’s called “isLocalUrl” or similar.

We added a NumSanitizer, because we can’t have SSRF just with numbers.

There is a library called Validator that we recommend to check the type of a variable. For example:

err := v10.New().validate.Var(tainted, "alpha")
if err == nil {
    http.Get(fmt.Sprintf("http://example.com/%v", tainted)) //OK
}

We added ValidatorAsSanitizer that looks for these checks and accepts the tagsf "alpha", "alphanum", "alphaunicode", "alphanumunicode", "number", "numeric" or “uuid”.

And we also added BodyTagSanitizer that identifies when a tag is set to a field in the body struct. Gin-gonic checks these and raises an error if the input has invalid type. The valid tags are the same as mentioned in ValidatorAsSanitizer.

var body struct {
safe    string `binding:"alphanum"`
}
err := c.ShouldBindJSON(&body)
if err == nil {
	http.Get(fmt.Sprintf("http://example.com/%s", body.safe)) // OK
}

• Are you planning to discuss this vulnerability submission publicly? (Blog Post, social networks, etc). We would love to have you spread the word about the good work you are doing

Result(s)

We have lots of cases, but all on private code. We’ll be sending the details to [email protected]

Collaborators

@luciaromeroML
@alangatti
@valeria-meli
@npesaresi

[valeria-meli]

Any news on this issue?

[ghsecuritylab]

Your submission is now in status CodeQL review.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed

[ghsecuritylab]

Your submission is now in status CodeQL review.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed

[ghsecuritylab]

Your submission is now in status FP Check.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed

[antonio-morales]

Any news on this issue?

Hi @valeria-meli,

first of all, my apologies for the delay.

The query is now in the "CodeQL review" stage, and we'll contact you soon to inform you about our final decision.

[ghsecuritylab]

Your submission is now in status SecLab finalize.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed

[ghsecuritylab]

Your submission is now in status Pay.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed

[xcorail]

Created Hackerone report 1391725 for bounty 347514 : [430-1] Yet another SSRF query for Go

[ghsecuritylab]

Your submission is now in status Closed.

For information, the evaluation workflow is the following:
SecLab review > Generate Query Results > FP Check > CodeQL review > SecLab finalize > Pay > Closed