Golang: Add query to detect CSRF vulnerabilities #532
Conversation
When a web server is designed to receive a request from a client without any mechanism for verifying that it was intentionally sent, then it might be possible for an attacker to trick a client into making an unintentional request to the web server which will be treated as an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can result in exposure of data or unintended code execution.
ghost
self-requested a review
as a 
ghost
mentioned this pull request
| // private class GorillaCsrf extends Sanitizer { | ||
| // GorillaCsrf() { | ||
| // exists(Method csrf, Method protect, int i | | ||
| // protect.hasQualifiedName("gorilla/csrf", "Protect") and | ||
| // protect.getResult(0) = csrf and | ||
| // csrf.getParameter(0) = this.asParameter() | ||
| // ) | ||
| // } | ||
| // } | ||
| // private class CustomCsrf extends Sanitizer { | ||
| // CustomCsrf() { | ||
| // exists(Method csrf, Method handler, int i | | ||
| // csrf.getName().regexpMatch(["*csrf*","*xsrf*"]) and | ||
| // handler.hasQualifiedName("gorilla/mux", ["Handler", "HandlerFunc"]) | | ||
| // handler.getAParameter() = csrf and | ||
| // handler.getACall().getTarget() = this.asExpr() | ||
| // ) | ||
| // } | ||
| // } |
There was a problem hiding this comment.
This code does not work. I want to detect a sanitizer like the one shown below:
CSRF := csrf.Protect([]byte("32-byte-long-auth-key"))
http.ListenAndServe(":8000", CSRF(r))The CSRF variable is a function returned by the Protect call.
I can't seem to figure out a way to track a flow this way.
How do I go about achieving this?
There was a problem hiding this comment.
Regarding your code that doesn't work:
Note DataFlow::CallNode already has getACalleeSource(), which does simple intraprocedural flow to find out what's providing the callee. Perhaps you simply want to sanitise the argument to a CallNode when that CallNode has a CalleeSource csrf.Protect?
Though note that I wouldn't expect these functions to be modelled in the first place, being from an external library, so I would expect that if r is tainted then csrf.Protect(r) would untainted in any case without having to define a sanitiser.
| */ | ||
| private class HandlerCreation extends Source { | ||
| HandlerCreation() { | ||
| exists(Method m | m.hasQualifiedName("net/http", ["Handle", "HandleFunc"]) | |
There was a problem hiding this comment.
These methods don't return anything. Do you mean to look for handlers flowing to Handle(...) and other functions that register a handler, without passing through some sort of CSRF wrapper first?
There was a problem hiding this comment.
@smowton Yes, I mean to look for that. But since, the arguments in this case are higher order functions I am unable to deal with them. As for the Csrf.Protect() the Protect call returns a function which is to be called in a manner shown below
CSRF := csrf.Protect([]byte("32-byte-long-auth-key"))(router)
Hence, tracking flow is becoming difficult for me here.
There was a problem hiding this comment.
What do you actually want the query to flag? Any request handler function that gets passed to http.ListenAndServe or similar without being wrapped in a CSRF layer produced by csrf.Protect?
If so I don't think you need to care about higher-order functions: source = callables with appropriate type for ListenAndServe; sink = argument to ListenAndServe; the call to CSRF, however it is protected, is sanitizing per default because the function returned from csrf.Protect is opaque to us; add additionalTaintSteps for any external functions that wrap a handler without providing CSRF protection, so e.g. addCachingLayer(r) is tainted if r is tainted.
There was a problem hiding this comment.
@smowton Yes, I totally get your point. I can't believe I missed this. I have removed the incorrect code now. PTAL.
|
Started an evaluation for the seclab. |
|
Given Seclab closed this for lack of results, it's probably not worth adding to our query pack right now. Please do get back to us if you're able to provide some CVE hits using this query. |
1 participant
When a web server is designed to receive a request from a client without any mechanism for verifying that it was intentionally sent, then it might be possible for an attacker to trick a client into making an unintentional request to the web server which will be treated as an authentic request. This can be done via a URL, image load, XMLHttpRequest, etc. and can result in exposure of data or unintended code execution.