hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-30 03:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

JavaScript: Improve StackTraceExposure query.

Browse Source 
It now also flags exposure of the entire exception object (not just the `stack` property).
This commit is contained in:
Max Schaefer
2018-10-31 10:32:42 -04:00
parent a7290e5aeb
commit bdfe938d02
4 changed files with 38 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files

22
javascript/ql/src/semmle/javascript/security/dataflow/StackTraceExposure.qll
View File

@@ -23,22 +23,32 @@ module StackTraceExposure {
src instanceof Source
}
override predicate isSanitizer(DataFlow::Node nd) {
super.isSanitizer(nd)
or
// read of a property other than `stack`
nd.(DataFlow::PropRead).getPropertyName() != "stack"
or
// `toString` does not include the stack trace
nd.(DataFlow::MethodCallNode).getMethodName() = "toString"
or
nd = StringConcatenation::getAnOperand(_)
}
override predicate isSink(DataFlow::Node snk) {
snk instanceof Sink
}
}
/**
* A read of the `stack` property of an exception, viewed as a data flow
* sink for stack trace exposure vulnerabilities.
*/
class DefaultSource extends Source, DataFlow::ValueNode {
class DefaultSource extends Source, DataFlow::Node {
DefaultSource() {
// any read of the `stack` property of an exception is a source
exists (Parameter exc |
exc = any(TryStmt try).getACatchClause().getAParameter() and
this = DataFlow::parameterNode(exc).getAPropertyRead("stack")
)
// any exception is a source
this = DataFlow::parameterNode(any(TryStmt try).getACatchClause().getAParameter())
}
}

4
javascript/ql/test/query-tests/Security/CWE-209/StackTraceExposure.expected
View File

@@ -1 +1,3 @@
| node.js:11:13:11:21 | err.stack | Stack trace information from $@ may be exposed to an external user here. | node.js:11:13:11:21 | err.stack | here |
| node.js:11:13:11:21 | err.stack | Stack trace information from $@ may be exposed to an external user here. | node.js:8:10:8:12 | err | here |
| tst.js:7:13:7:13 | e | Stack trace information from $@ may be exposed to an external user here. | tst.js:6:12:6:12 | e | here |
| tst.js:17:11:17:17 | e.stack | Stack trace information from $@ may be exposed to an external user here. | tst.js:6:12:6:12 | e | here |

18
javascript/ql/test/query-tests/Security/CWE-209/tst.js Normal file
View File

@@ -0,0 +1,18 @@
var http = require('http');
http.createServer(function onRequest(req, res) {
try {
throw new Error();
} catch (e) {
res.end(e); // NOT OK
fail(res, e);
res.end(e.message); // OK
res.end("Caught exception " + e); // OK
res.end(e.toString()); // OK
res.end(`Caught exception ${e}.`); // OK
}
});
function fail(res, e) {
res.end(e.stack); // NOT OK
}