hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-03-17 04:56:58 +01:00
Code Issues Packages Projects Releases Wiki Activity

add additional taint steps when type-tracking RemoteFlowSource

Browse Source
This commit is contained in:
Erik Krogh Kristensen
2020-06-16 14:55:07 +02:00
parent b6b838774e
commit a0951f76b6
3 changed files with 17 additions and 4 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql
View File

@@ -20,17 +20,22 @@ private import semmle.javascript.security.dataflow.CodeInjectionCustomizations
/**
* Gets a type-tracked instance of `RemoteFlowSource` using type-tracker `t`.
*/
private DataFlow::SourceNode remoteFlow(DataFlow::TypeTracker t) {
private DataFlow::Node remoteFlow(DataFlow::TypeTracker t) {
t.start() and
result instanceof RemoteFlowSource
or
exists(DataFlow::TypeTracker t2 | result = remoteFlow(t2).track(t2, t))
exists(DataFlow::TypeTracker t2, DataFlow::Node prev | prev = remoteFlow(t2) |
t2 = t.smallstep(prev, result)
or
any(TaintTracking::AdditionalTaintStep dts).step(prev, result) and
t = t2
)
}
/**
* Gets a type-tracked reference to a `RemoteFlowSource`.
*/
private DataFlow::SourceNode remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
private DataFlow::Node remoteFlow() { result = remoteFlow(DataFlow::TypeTracker::end()) }
/**
* Gets a type-back-tracked instance of a code-injection sink using type-tracker `t`.
@@ -60,7 +65,7 @@ where
// Basic detection of duplicate results with `js/code-injection`.
not (
sink.getNode().(StringOps::ConcatenationLeaf).getRoot() = endsInCodeInjectionSink() and
remoteFlow().flowsTo(source.getNode().(DataFlow::InvokeNode).getAnArgument())
remoteFlow() = source.getNode().(DataFlow::InvokeNode).getAnArgument()
)
select sink.getNode(), source, sink, "$@ flows to here and is used to construct code.",
source.getNode(), "Improperly sanitized value"

4
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/ImproperCodeSanitization.expected
View File

@@ -31,6 +31,9 @@ nodes
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
edges
| bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` | bad-code-sanitization.js:7:31:7:43 | safeProp(key) |
| bad-code-sanitization.js:2:65:2:90 | `[${JSO ... key)}]` | bad-code-sanitization.js:2:12:2:90 | /^[_$a- ... key)}]` |
@@ -49,6 +52,7 @@ edges
| bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) | bad-code-sanitization.js:44:22:44:42 | JSON.st ... (input) |
| bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) | bad-code-sanitization.js:52:28:52:62 | JSON.st ... bble")) |
| bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) | bad-code-sanitization.js:54:29:54:63 | JSON.st ... bble")) |
| bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) | bad-code-sanitization.js:58:29:58:49 | JSON.st ... (taint) |
#select
| bad-code-sanitization.js:8:27:8:46 | statements.join(';') | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | bad-code-sanitization.js:8:27:8:46 | statements.join(';') | $@ flows to here and is used to construct code. | bad-code-sanitization.js:2:69:2:87 | JSON.stringify(key) | Improperly sanitized value |
| bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | $@ flows to here and is used to construct code. | bad-code-sanitization.js:15:44:15:63 | htmlescape(pathname) | Improperly sanitized value |

4
javascript/ql/test/query-tests/Security/CWE-094/CodeInjection/bad-code-sanitization.js
View File

@@ -52,4 +52,8 @@ app.get('/some/path', function(req, res) {
var foo = `(function(){${JSON.stringify(req.param("wobble"))}))` // NOT - the source is remote-flow, but we know of no sink.
setTimeout(`(function(){${JSON.stringify(req.param("wobble"))}))`); // OK - the source is remote-flow, and the sink is code-injection.
var taint = [req.body.name, "foo"].join("\n");
setTimeout(`(function(){${JSON.stringify(taint)}))`); // OK - the source is remote-flow, and the sink is code-injection.
});