From 5aec9ebe40c86edcae7831ff8f25284489fb116f Mon Sep 17 00:00:00 2001 From: Esben Sparre Andreasen Date: Tue, 17 Nov 2020 20:41:45 +0100 Subject: [PATCH] JS: sanitize taint in the LHS of && --- .../ql/src/semmle/javascript/dataflow/TaintTracking.qll | 3 ++- .../library-tests/TaintTracking/BasicTaintTracking.expected | 2 ++ .../library-tests/TaintTracking/DataFlowTracking.expected | 3 +++ javascript/ql/test/library-tests/TaintTracking/tst.js | 4 ++++ 4 files changed, 11 insertions(+), 1 deletion(-) diff --git a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll index becd6b4515d0..ad39edb2912f 100644 --- a/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll +++ b/javascript/ql/src/semmle/javascript/dataflow/TaintTracking.qll @@ -83,7 +83,8 @@ module TaintTracking { /** Holds if the edge from `pred` to `succ` is a taint sanitizer for data labelled with `lbl`. */ predicate isSanitizerEdge(DataFlow::Node pred, DataFlow::Node succ, DataFlow::FlowLabel lbl) { - none() + // sanitize the falsy LHS of `LHS && RHS` by default + pred.asExpr() = any(LogAndExpr e).getLeftOperand() and lbl.isTaint() } /** diff --git a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected index e1383b7f9daf..8090d81e4dd4 100644 --- a/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected +++ b/javascript/ql/test/library-tests/TaintTracking/BasicTaintTracking.expected @@ -143,3 +143,5 @@ typeInferenceMismatch | tst.js:2:13:2:20 | source() | tst.js:45:10:45:24 | x.map(x2 => x2) | | tst.js:2:13:2:20 | source() | tst.js:47:10:47:30 | Buffer. ... 'hex') | | tst.js:2:13:2:20 | source() | tst.js:48:10:48:22 | new Buffer(x) | +| tst.js:2:13:2:20 | source() | tst.js:51:7:51:12 | x && x | +| tst.js:2:13:2:20 | source() | tst.js:52:7:52:12 | y && x | diff --git a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected index a304345d522e..5e2b15f99134 100644 --- a/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected +++ b/javascript/ql/test/library-tests/TaintTracking/DataFlowTracking.expected @@ -79,3 +79,6 @@ | thisAssignments.js:4:17:4:24 | source() | thisAssignments.js:5:10:5:18 | obj.field | | thisAssignments.js:7:19:7:26 | source() | thisAssignments.js:8:10:8:20 | this.field2 | | tst.js:2:13:2:20 | source() | tst.js:4:10:4:10 | x | +| tst.js:2:13:2:20 | source() | tst.js:50:7:50:12 | x && y | +| tst.js:2:13:2:20 | source() | tst.js:51:7:51:12 | x && x | +| tst.js:2:13:2:20 | source() | tst.js:52:7:52:12 | y && x | diff --git a/javascript/ql/test/library-tests/TaintTracking/tst.js b/javascript/ql/test/library-tests/TaintTracking/tst.js index 61613507b9ee..8cdbe1702d6a 100644 --- a/javascript/ql/test/library-tests/TaintTracking/tst.js +++ b/javascript/ql/test/library-tests/TaintTracking/tst.js @@ -46,4 +46,8 @@ function test() { sink(Buffer.from(x, 'hex')); // NOT OK sink(new Buffer(x)); // NOT OK + + sink(x && y); // OK + sink(x && x); // NOT OK + sink(y && x); // NOT OK }