Add some queries for commonly-used third-part libraries #166
Conversation
There was a problem hiding this comment.
Many thanks for your contribution, we really appreciate it!
I have a few initial thoughts below.
Since this is a contribution to the standard library, we need to hold it to a higher standard than an experimental contribution. In particular, it would be great if you could add tests for all the new APIs you added support for. @sauyon can help with stubbing out dependencies, you can take a look at a few recent PRs to see how this works.
Also, can you make sure that all your code is auto-formatted?
| @@ -0,0 +1,25 @@ | ||
| /** |
There was a problem hiding this comment.
Can you move this to frameworks/Encoding.qll, please? No nee for the "thirdpartlib" bit.
| @@ -0,0 +1,25 @@ | ||
| /** | ||
| * Provides classes modeling security-relevant aspects of the third-part libraries. |
There was a problem hiding this comment.
This comment is too generic; can you make it a little more specific?
|
|
||
| import go | ||
|
|
||
| module ThirdPartEncodingJson { |
There was a problem hiding this comment.
No need to wrap this into a module, I think.
|
|
||
| module ThirdPartEncodingJson { | ||
| /** Provides models of some functions in the `github.com/json-iterator/go` package. */ | ||
| class JsoniterUnmarshalingFunction extends TaintTracking::FunctionModel, UnmarshalingFunction::Range { |
There was a problem hiding this comment.
| class JsoniterUnmarshalingFunction extends TaintTracking::FunctionModel, UnmarshalingFunction::Range { | |
| private class JsoniterUnmarshalingFunction extends TaintTracking::FunctionModel, UnmarshalingFunction::Range { |
| import go | ||
|
|
||
| module ThirdPartEncodingJson { | ||
| /** Provides models of some functions in the `github.com/json-iterator/go` package. */ |
There was a problem hiding this comment.
Again, a more specific comment would be better, since this models one particular function.
| * Source from go-resultful | ||
| * Document: https://github.com/emicklei/go-restful | ||
| */ | ||
| class GoRestfulSource extends DataFlow::Node, UntrustedFlowSource::Range { |
There was a problem hiding this comment.
I'd suggest making this class private and moving it into go/frameworks/HTTP.qll.
| exists( | ||
| Method meth, string name | | ||
| meth.hasQualifiedName("github.com/emicklei/go-restful", "Request", name) and | ||
| asExpr() = meth.getACall().asExpr() and |
There was a problem hiding this comment.
| asExpr() = meth.getACall().asExpr() and | |
| this = meth.getACall() and |
| GoRestfulSource() { | ||
| exists( | ||
| Method meth, string name | | ||
| meth.hasQualifiedName("github.com/emicklei/go-restful", "Request", name) and |
There was a problem hiding this comment.
You can use the new(-ish) [...] syntax for this:
| meth.hasQualifiedName("github.com/emicklei/go-restful", "Request", name) and | |
| meth.hasQualifiedName(package("github.com/emicklei/go-restful", ""), "Request", [ | |
| "QueryParameters", "QueryParameter", | |
| "BodyParameter", "HeaderParameter", | |
| "PathParameters", "PathParameter" | |
| ]) and |
(Note also the use of the package helper predicate; go-restful seems to use semantic import versioning.)
There was a problem hiding this comment.
Sorry for hijacking this.
Is there a "coding style" for set literals?
Is it ok to use them in functions like this hasQualifiedName(_, ["foo", "bar", "foobar"]) (1)
or should one rather use something like this instead (2)
exists(string name | name in ["foo", "bar", "foobar"] | hasQualifiedName(_, name))If one already knows set literals (1) is probably understandable, but if I didn't know them I'd probably find that confusing.
There was a problem hiding this comment.
There isn't, both alternatives are fine.
| @@ -0,0 +1,48 @@ | ||
| /** |
There was a problem hiding this comment.
Can you please make these classes private and move them into go.frameworks.SQL?
There was a problem hiding this comment.
Same suggestions regarding code style as with GoRestfulSource above.
|
Superseded by #270. |
Hi, I add some queries for commonly-used third-part libraries: