added sql injection rules for node express#8
added sql injection rules for node express#8Newbi3Fix3r wants to merge 1 commit intomobb-dev:mainfrom
Conversation
| metavariable: $SQL | ||
| patterns: | ||
| - pattern-regex: .*\b(?i)(SELECT|CREATE|DROP|INSERT|DELETE|UPDATE|ALTER)\b.* | ||
| fix: $CONN.escape($REQ.body.$DATA) |
There was a problem hiding this comment.
why only body parameters? I guess it can be req.params, req.cookies, req.headers, req.query or req.body
| - metavariable-pattern: | ||
| metavariable: $SQL | ||
| patterns: | ||
| - pattern-regex: .*\b(?i)(SELECT|CREATE|DROP|INSERT|DELETE|UPDATE|ALTER)\b.* |
There was a problem hiding this comment.
isn't it enough that that we use "$CONN.query($SQL,...)"? why do we need to validate it starts with an SQL command? Just want to save the performance of executing another regex and it seems it doesn't add much value?
| Potential SQL Injection identified. Consider using the connection.escape method built in node-mysql | ||
| patterns: | ||
| - pattern-inside: | | ||
| $MYSQL=require('mysql') |
There was a problem hiding this comment.
there are other require/import styles (can look at cookie attribute rules for reference) that can be used in JS/TS. We should probably include all styles or just not include the "require" in the rule.
| rules: | ||
| - id: nodejs-mysql-body-inection | ||
| languages: | ||
| - javascript |
There was a problem hiding this comment.
we should probably also use this rule for typescript as well?
| - pattern-inside: $APP.options(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-regex: \+.*?\+ | ||
| - pattern-not-regex: \+.*?escape.*?\+ | ||
| - pattern: $REQ.body.$DATA |
There was a problem hiding this comment.
why only body parameters? I guess it can be req.params, req.cookies, req.headers, req.query or req.body
| - pattern-inside: $APP.head(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.delete(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.options(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-regex: \+.*?\+ |
There was a problem hiding this comment.
What about only a single plus from the right or from the left
| - pattern-inside: $APP.head(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.delete(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.options(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-regex: \+.*?\+ |
There was a problem hiding this comment.
I think it can work with a non regex rule for performance. something like: pattern: ... + $REQ.body.$DATA + ...
| - pattern-inside: $APP.head(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.delete(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-inside: $APP.options(..., function $FUNC($REQ, $RES) {...}) | ||
| - pattern-regex: \+.*?\+ |
There was a problem hiding this comment.
it is more complete to have also a variant like this:
- pattern-inside: |
$STR = ... + $REQ.body.$DATA + ...;
...
-pattern: ... + $STR + ...
No description provided.