Spring: Add Semgrep metavar for exact path/route variables #11
Conversation
…in:/usr/local/sbin:/usr/local/opt/[email protected]/bin:/Users/gabrielmarquet/.nvm/versions/node/v17.8.0/bin:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/opt:/Users/gabrielmarquet/Library/Python/2.7/bin:/Users/gabrielmarquet/Library/Python/3.8/bin:/Users/gabrielmarquet/go/bin:/Users/gabrielmarquet/.local/bin metavar to spring rules and use them in D3 viz
…add HTTP Method/Verb to types
I'm guessing this is this bug: semgrep/semgrep#8120 |
|
Correct ! |
|
The parallelism bug is gone, but now I'm having a weird issue. |
| @property | ||
| def rd_route(self): | ||
| if "$PATH" in self.metavars: #For some rules we have extracted a path in the $PATH metavar | ||
| return "{} {}".format(self.rd_method, self.metavars['$PATH']['abstract_content']) |
There was a problem hiding this comment.
| return "{} {}".format(self.rd_method, self.metavars['$PATH']['abstract_content']) | |
| return "{} {}".format(self.rd_method, self.metavar_content('$PATH') |
| @property | ||
| def rd_method(self): | ||
| # split by space and deduplicate and return first element | ||
| return list(set(self.metavars['$METHOD']['abstract_content'].split(' ')))[0] |
There was a problem hiding this comment.
| return list(set(self.metavars['$METHOD']['abstract_content'].split(' ')))[0] | |
| return list(set(self.metavar_content('$METHOD').split(' ')))[0] |
There was a problem hiding this comment.
Why split on ' ' here and take the first element? It seems cleaner to me to just use the $METHOD content.
| @@ -150,7 +150,8 @@ rules: | |||
| patterns: | |||
| - pattern: | | |||
| @$METHOD | |||
| $RETURNTYPE $FUNC(...) { ... } | |||
| @Path($PATH) | |||
There was a problem hiding this comment.
Hmm, I vaguely remember that these Path annotations are optional, so this will break some existing findings: https://docs.oracle.com/javaee/6/api/javax/ws/rs/Path.html.
Generally I like the idea of using a consistent metavariables (e.g. $PATH, $METHOD, etc.) across all languages and frameworks, but I think they have to be optional in terms of detection. Many frameworks have an implicit default in certain cases, and we don't want to miss detecting routes because we're expecting explicit values everywhere.
I'm not sure how best to architect these rules such that we can easily find routes and their optional path, method, etc, information.
There was a problem hiding this comment.
I think that explains why the tests are failing, probably because the rule I modified is breaking some detections
Either we make very general rules without too many metavariables, so that we cover widely the detected variations of code, and then refine (using regex or whatever, I thought about it before)
Or we can figure out a way to make the rules more specialized, versions with and without the metavars, and fix the test examples
I believe this means there is a test with a |
|
Sorry, for the delay, will take a look at this asap |
Attempt at fixing/improving #10 and #12
Adding a $PATH metavar in the rules allows you to really extract the exact values
Only on unauthorized paths for now, I tried also supporting that for the other rules but it would break weirdly
The final graph will show something more readable like:
(Can't run the tests on my machine, weird python parallel processing issues)