reintroduce the number sinks

This commit is contained in:
Erik Krogh Kristensen
2022-04-12 16:26:10 +02:00
parent 029459cc35
commit e2b7f7d05d
6 changed files with 93 additions and 39 deletions

View File

@@ -77,4 +77,59 @@ module ResourceExhaustion {
result = "This creates a timer with a user-controlled duration"
}
}
/**
* A node that determines the size of a buffer, considered as a data flow sink for resource exhaustion vulnerabilities.
*/
class BufferSizeSink extends Sink {
BufferSizeSink() {
exists(DataFlow::SourceNode clazz, DataFlow::InvokeNode invk, int index |
clazz = DataFlow::globalVarRef("Buffer") and this = invk.getArgument(index)
|
exists(string name |
invk = clazz.getAMemberCall(name) and
(
name = "from" and index = 2 // the length argument
or
name = ["alloc", "allocUnsafe", "allocUnsafeSlow"] and index = 0 // the buffer size
)
)
or
invk = clazz.getAnInvocation() and
(
invk.getNumArgument() = 1 and // `new Buffer(size)`, it's only an issue if the size is a number, which we don't track precisely.
index = 0
or
invk.getNumArgument() = 3 and index = 2 // the length argument
)
)
or
this = DataFlow::globalVarRef("SlowBuffer").getAnInstantiation().getArgument(0)
}
override string getProblemDescription() {
result = "This creates a buffer with a user-controlled size"
}
}
/**
* A node that determines the size of an array, considered as a data flow sink for resource exhaustion vulnerabilities.
* This is only an issue if the argument is a number, which we don't track precisely.
*/
class DenseArraySizeSink extends Sink {
DenseArraySizeSink() {
// Arrays are sparse by default, so we must also look at how the array is used
exists(DataFlow::ArrayConstructorInvokeNode instance |
this = instance.getArgument(0) and
instance.getNumArgument() = 1
|
exists(instance.getAMethodCall(["map", "fill", "join", "toString"])) or
instance.flowsToExpr(any(AddExpr p).getAnOperand())
)
}
override string getProblemDescription() {
result = "This creates an array with a user-controlled length"
}
}
}

View File

@@ -12,7 +12,7 @@
import javascript
import DataFlow::PathGraph
import experimental.semmle.javascript.security.dataflow.ResourceExhaustionQuery
import semmle.javascript.security.dataflow.ResourceExhaustionQuery
from Configuration dataflow, DataFlow::PathNode source, DataFlow::PathNode sink
where dataflow.hasFlowPath(source, sink)

View File

@@ -0,0 +1,3 @@
import javascript
import semmle.javascript.security.dataflow.ResourceExhaustionQuery
import testUtilities.ConsistencyChecking

View File

@@ -1,7 +1,3 @@
// this file contains many `NOT OK [INCONSISTENCY]` annotations, those
// would be resolved if the query used flow labels to recognice
// numbers flowing to sinks
var http = require("http"),
url = require("url");
@@ -13,73 +9,73 @@ var server = http.createServer(function(req, res) {
Buffer.from(n); // OK
Buffer.from(x, n); // OK
Buffer.from(x, y, s); // NOT OK
Buffer.from(x, y, n); // NOT OK [INCONSISTENCY]
Buffer.from(x, y, n); // NOT OK [INCONSISTENCY]
Buffer.alloc(n); // NOT OK [INCONSISTENCY]
Buffer.allocUnsafe(n); // NOT OK [INCONSISTENCY]
Buffer.allocUnsafeSlow(n); // NOT OK [INCONSISTENCY]
Buffer.from(x, y, n); // NOT OK
Buffer.from(x, y, n); // NOT OK
Buffer.alloc(n); // NOT OK
Buffer.allocUnsafe(n); // NOT OK
Buffer.allocUnsafeSlow(n); // NOT OK
new Buffer(n); // NOT OK [INCONSISTENCY]
new Buffer(n); // NOT OK
new Buffer(x, n); // OK
new Buffer(x, y, n); // NOT OK [INCONSISTENCY]
new Buffer(x, y, n); // NOT OK
new SlowBuffer(n); // NOT OK [INCONSISTENCY]
new SlowBuffer(n); // NOT OK
Array(n); // OK
new Array(n); // OK
Array(n).map(); // NOT OK [INCONSISTENCY]
new Array(n).map(); // NOT OK [INCONSISTENCY]
Array(n).fill(); // NOT OK [INCONSISTENCY]
Array(n).join(); // NOT OK [INCONSISTENCY]
Array(n).toString(); // NOT OK [INCONSISTENCY]
Array(n) + x; // NOT OK [INCONSISTENCY]
Array(n).map(); // NOT OK
new Array(n).map(); // NOT OK
Array(n).fill(); // NOT OK
Array(n).join(); // NOT OK
Array(n).toString(); // NOT OK
Array(n) + x; // NOT OK
x.repeat(n); // NOT OK
x.repeat(s); // NOT OK
new Buffer(n * x); // NOT OK [INCONSISTENCY]
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
new Buffer(n * x); // NOT OK
new Buffer(n + n); // NOT OK
new Buffer(n + x); // OK (maybe) - but still flagged [INCONSISTENCY]
new Buffer(n + s); // OK (this is a string if `s` is a string) - but still flagged [INCONSISTENCY]
new Buffer(s + 2); // OK (this is a string if `s` is a string) - but still flagged [INCONSISTENCY]
new Buffer(s + s); // OK - but still flagged [INCONSISTENCY]
new Buffer(n + "X"); // OK - but still flagged [INCONSISTENCY]
new Buffer(Math.ceil(s)); // NOT OK [INCONSISTENCY]
new Buffer(Number(s)); // NOT OK [INCONSISTENCY]
new Buffer(Math.ceil(s)); // NOT OK
new Buffer(Number(s)); // NOT OK
new Buffer(new Number(s)); // OK
new Buffer(s + x.length); // OK (this is a string if `s` is a string)
new Buffer(s.length); // NOT OK [INCONSISTENCY]
new Buffer(s + x.length); // OK (this is a string if `s` is a string) - but still flagged [INCONSISTENCY]
new Buffer(s.length); // NOT OK
if (n < 100) {
new Buffer(n); // OK
} else {
new Buffer(n); // NOT OK [INCONSISTENCY]
new Buffer(n); // NOT OK
}
let ns = x ? n : s;
new Buffer(ns); // NOT OK [INCONSISTENCY]
new Buffer(ns); // NOT OK
new Buffer(n.toString()); // OK
new Buffer(n.toString()); // OK - but still flagged [INCONSISTENCY]
if (typeof n === "string") {
new Buffer(n); // OK
new Buffer(n); // OK - but still flagged [INCONSISTENCY]
} else {
new Buffer(n); // NOT OK [INCONSISTENCY]
new Buffer(n); // NOT OK
}
if (typeof n === "number") {
new Buffer(n); // NOT OK [INCONSISTENCY]
new Buffer(n); // NOT OK
} else {
new Buffer(n); // OK
new Buffer(n); // OK - but still flagged [INCONSISTENCY]
}
if (typeof s === "number") {
new Buffer(s); // NOT OK [INCONSISTENCY]
new Buffer(s); // NOT OK
} else {
new Buffer(s); // OK
new Buffer(s); // OK - but stil flagged [INCONSISTENCY]
}
setTimeout(f, n); // NOT OK