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

JS: avoid type inference in the taint steps (just a nice to have)

Browse Source
This commit is contained in:
Esben Sparre Andreasen
2020-06-15 14:57:59 +02:00
parent fa4e8914e6
commit d9d8eb4805
3 changed files with 37 additions and 90 deletions
Download Patch File Download Diff File Expand all files Collapse all files

78
javascript/ql/src/Security/CWE-770/MemoryExhaustion.ql
View File

@@ -66,52 +66,15 @@ class Configuration extends TaintTracking::Configuration {
DataFlow::Node src, DataFlow::Node dst, DataFlow::FlowLabel srclabel,
DataFlow::FlowLabel dstlabel
) {
exists(Expr dstExpr, Expr srcExpr | dstExpr = dst.asExpr() and srcExpr = src.asExpr() |
// reuse taint steps
super.isAdditionalFlowStep(src, dst) and
(
srclabel = dstlabel and
not dstExpr instanceof AddExpr and
not dst.(DataFlow::MethodCallNode).calls(src, "toString")
or
dstlabel.isTaint() and
dst.(DataFlow::MethodCallNode).calls(src, "toString")
or
// this conversion step is probably covered below
dstlabel instanceof Label::Number and dst.(AnalyzedNode).getTheType() = TTNumber()
)
or
//
// steps that introduce or preserve a number
dstlabel instanceof Label::Number and
(
dst.(DataFlow::PropRead).accesses(src, ["length", "size"])
or
dstExpr.(BinaryExpr).getAnOperand() = srcExpr and
not dstExpr instanceof AddExpr
or
dstExpr.(PlusExpr).getOperand() = srcExpr
or
exists(DataFlow::CallNode c |
c = dst and
src = c.getAnArgument()
|
c = DataFlow::globalVarRef("Math").getAMemberCall(_) or
c = DataFlow::globalVarRef(["Number", "parseInt", "parseFloat"]).getACall()
)
)
or
// optimistic propagation through plus if either operand is a number
exists(Expr operand | dstExpr.(AddExpr).hasOperands(operand, srcExpr) |
operand.analyze().getTheType() = TTNumber()
or
operand.flow().getALocalSource().(DataFlow::PropRead).getPropertyName() = "length"
or
srclabel instanceof Label::Number and
// unless the result provably is a string
not operand.analyze().getTheType() = TTString()
)
)
dstlabel instanceof Label::Number and
isNumericFlowStep(src, dst)
or
// reuse taint steps
super.isAdditionalFlowStep(src, dst) and
not dst.asExpr() instanceof AddExpr and
if dst.(DataFlow::MethodCallNode).calls(src, "toString")
then dstlabel.isTaint()
else srclabel = dstlabel
}
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode guard) {
@@ -121,6 +84,29 @@ class Configuration extends TaintTracking::Configuration {
}
}
predicate isNumericFlowStep(DataFlow::Node src, DataFlow::Node dst) {
// steps that introduce or preserve a number
dst.(DataFlow::PropRead).accesses(src, ["length", "size"])
or
exists(DataFlow::CallNode c |
c = dst and
src = c.getAnArgument()
|
c = DataFlow::globalVarRef("Math").getAMemberCall(_) or
c = DataFlow::globalVarRef(["Number", "parseInt", "parseFloat"]).getACall()
)
or
exists(Expr dstExpr, Expr srcExpr |
dstExpr = dst.asExpr() and
srcExpr = src.asExpr()
|
dstExpr.(BinaryExpr).getAnOperand() = srcExpr and
not dstExpr instanceof AddExpr
or
dstExpr.(PlusExpr).getOperand() = srcExpr
)
}
/**
* A sanitizer that blocks taint flow if the size of a number is limited.
*/

39
javascript/ql/test/query-tests/Security/CWE-770/MemoryExhaustion.expected
View File

@@ -43,28 +43,12 @@ nodes
| memory-exhaustion.js:38:14:38:14 | n |
| memory-exhaustion.js:38:14:38:18 | n * x |
| memory-exhaustion.js:38:14:38:18 | n * x |
| memory-exhaustion.js:39:14:39:14 | n |
| memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:39:18:39:18 | n |
| memory-exhaustion.js:40:14:40:14 | n |
| memory-exhaustion.js:40:14:40:18 | n + x |
| memory-exhaustion.js:40:14:40:18 | n + x |
| memory-exhaustion.js:41:14:41:14 | n |
| memory-exhaustion.js:41:14:41:18 | n + s |
| memory-exhaustion.js:41:14:41:18 | n + s |
| memory-exhaustion.js:42:14:42:14 | s |
| memory-exhaustion.js:42:14:42:18 | s + 2 |
| memory-exhaustion.js:42:14:42:18 | s + 2 |
| memory-exhaustion.js:46:14:46:25 | Math.ceil(s) |
| memory-exhaustion.js:46:14:46:25 | Math.ceil(s) |
| memory-exhaustion.js:46:24:46:24 | s |
| memory-exhaustion.js:47:14:47:22 | Number(s) |
| memory-exhaustion.js:47:14:47:22 | Number(s) |
| memory-exhaustion.js:47:21:47:21 | s |
| memory-exhaustion.js:50:14:50:14 | s |
| memory-exhaustion.js:50:14:50:25 | s + x.length |
| memory-exhaustion.js:50:14:50:25 | s + x.length |
| memory-exhaustion.js:51:14:51:14 | s |
| memory-exhaustion.js:51:14:51:21 | s.length |
| memory-exhaustion.js:51:14:51:21 | s.length |
@@ -83,10 +67,8 @@ edges
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:7:20:7:20 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:36:12:36:12 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:36:12:36:12 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:42:14:42:14 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:46:24:46:24 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:47:21:47:21 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:50:14:50:14 | s |
| memory-exhaustion.js:6:7:6:42 | s | memory-exhaustion.js:51:14:51:14 | s |
| memory-exhaustion.js:6:11:6:34 | url.par ... , true) | memory-exhaustion.js:6:11:6:40 | url.par ... ).query |
| memory-exhaustion.js:6:11:6:40 | url.par ... ).query | memory-exhaustion.js:6:11:6:42 | url.par ... query.s |
@@ -124,10 +106,6 @@ edges
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:35:12:35:12 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:35:12:35:12 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:38:14:38:14 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:39:14:39:14 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:39:18:39:18 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:40:14:40:14 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:41:14:41:14 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:56:16:56:16 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:56:16:56:16 | n |
| memory-exhaustion.js:7:7:7:21 | n | memory-exhaustion.js:59:16:59:16 | n |
@@ -139,22 +117,10 @@ edges
| memory-exhaustion.js:7:20:7:20 | s | memory-exhaustion.js:7:11:7:21 | parseInt(s) |
| memory-exhaustion.js:38:14:38:14 | n | memory-exhaustion.js:38:14:38:18 | n * x |
| memory-exhaustion.js:38:14:38:14 | n | memory-exhaustion.js:38:14:38:18 | n * x |
| memory-exhaustion.js:39:14:39:14 | n | memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:39:14:39:14 | n | memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:39:18:39:18 | n | memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:39:18:39:18 | n | memory-exhaustion.js:39:14:39:18 | n + n |
| memory-exhaustion.js:40:14:40:14 | n | memory-exhaustion.js:40:14:40:18 | n + x |
| memory-exhaustion.js:40:14:40:14 | n | memory-exhaustion.js:40:14:40:18 | n + x |
| memory-exhaustion.js:41:14:41:14 | n | memory-exhaustion.js:41:14:41:18 | n + s |
| memory-exhaustion.js:41:14:41:14 | n | memory-exhaustion.js:41:14:41:18 | n + s |
| memory-exhaustion.js:42:14:42:14 | s | memory-exhaustion.js:42:14:42:18 | s + 2 |
| memory-exhaustion.js:42:14:42:14 | s | memory-exhaustion.js:42:14:42:18 | s + 2 |
| memory-exhaustion.js:46:24:46:24 | s | memory-exhaustion.js:46:14:46:25 | Math.ceil(s) |
| memory-exhaustion.js:46:24:46:24 | s | memory-exhaustion.js:46:14:46:25 | Math.ceil(s) |
| memory-exhaustion.js:47:21:47:21 | s | memory-exhaustion.js:47:14:47:22 | Number(s) |
| memory-exhaustion.js:47:21:47:21 | s | memory-exhaustion.js:47:14:47:22 | Number(s) |
| memory-exhaustion.js:50:14:50:14 | s | memory-exhaustion.js:50:14:50:25 | s + x.length |
| memory-exhaustion.js:50:14:50:14 | s | memory-exhaustion.js:50:14:50:25 | s + x.length |
| memory-exhaustion.js:51:14:51:14 | s | memory-exhaustion.js:51:14:51:21 | s.length |
| memory-exhaustion.js:51:14:51:14 | s | memory-exhaustion.js:51:14:51:21 | s.length |
| memory-exhaustion.js:59:7:59:20 | ns | memory-exhaustion.js:60:14:60:15 | ns |
@@ -179,13 +145,8 @@ edges
| memory-exhaustion.js:35:12:35:12 | n | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:35:12:35:12 | n | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:36:12:36:12 | s | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:36:12:36:12 | s | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:38:14:38:18 | n * x | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:38:14:38:18 | n * x | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:39:14:39:18 | n + n | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:39:14:39:18 | n + n | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:40:14:40:18 | n + x | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:40:14:40:18 | n + x | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:41:14:41:18 | n + s | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:41:14:41:18 | n + s | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:42:14:42:18 | s + 2 | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:42:14:42:18 | s + 2 | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:46:14:46:25 | Math.ceil(s) | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:46:14:46:25 | Math.ceil(s) | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:47:14:47:22 | Number(s) | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:47:14:47:22 | Number(s) | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:50:14:50:25 | s + x.length | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:50:14:50:25 | s + x.length | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:51:14:51:21 | s.length | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:51:14:51:21 | s.length | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:56:16:56:16 | n | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:56:16:56:16 | n | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |
| memory-exhaustion.js:60:14:60:15 | ns | memory-exhaustion.js:6:21:6:27 | req.url | memory-exhaustion.js:60:14:60:15 | ns | This allocates an object with a user-controlled size from $@. | memory-exhaustion.js:6:21:6:27 | req.url | here |

10
javascript/ql/test/query-tests/Security/CWE-770/memory-exhaustion.js
View File

@@ -36,10 +36,10 @@ var server = http.createServer(function(req, res) {
x.repeat(s); // NOT OK
new Buffer(n * x); // NOT OK
new Buffer(n + n); // NOT OK
new Buffer(n + x); // NOT OK (maybe)
new Buffer(n + s); // OK [INCONSISTENCY]: this is a string if `s` is a string
new Buffer(s + 2); // OK [INCONSISTENCY]: this is a string if `s` is a string
new Buffer(n + n); // NOT OK [INCONSISTENCY]
new Buffer(n + x); // OK (maybe)
new Buffer(n + s); // OK (this is a string if `s` is a string)
new Buffer(s + 2); // OK (this is a string if `s` is a string)
new Buffer(s + s); // OK
new Buffer(n + "X"); // OK
@@ -47,7 +47,7 @@ var server = http.createServer(function(req, res) {
new Buffer(Number(s)); // NOT OK
new Buffer(new Number(s)); // OK
new Buffer(s + x.length); // OK [INCONSISTENCY]: this is a string if `s` is a string
new Buffer(s + x.length); // OK (this is a string if `s` is a string)
new Buffer(s.length); // NOT OK
if (n < 100) {