diff --git a/javascript/change-notes/2021-05-18-clone.md b/javascript/change-notes/2021-05-18-clone.md new file mode 100644 index 00000000000..42d52d72a3d --- /dev/null +++ b/javascript/change-notes/2021-05-18-clone.md @@ -0,0 +1,4 @@ +lgtm,codescanning +* The dataflow libraries now model dataflow in the `clone` library. + Affected packages are + [clone](https://npmjs.com/package/clone) diff --git a/javascript/ql/src/semmle/javascript/Extend.qll b/javascript/ql/src/semmle/javascript/Extend.qll index 9b7255983b9..d50054fc4ee 100644 --- a/javascript/ql/src/semmle/javascript/Extend.qll +++ b/javascript/ql/src/semmle/javascript/Extend.qll @@ -174,3 +174,17 @@ private class ExtendCallTaintStep extends TaintTracking::SharedTaintStep { ) } } + +private import semmle.javascript.dataflow.internal.PreCallGraphStep + +/** + * A step for the `clone` package. + */ +private class CloneStep extends PreCallGraphStep { + override predicate step(DataFlow::Node pred, DataFlow::Node succ) { + exists(DataFlow::CallNode call | call = DataFlow::moduleImport("clone").getACall() | + pred = call.getArgument(0) and + succ = call + ) + } +} diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected index f8ceeb45b62..e8d093325ad 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected @@ -174,6 +174,14 @@ nodes | tst2.js:18:12:18:12 | p | | tst2.js:21:14:21:14 | p | | tst2.js:21:14:21:14 | p | +| tst2.js:30:7:30:24 | p | +| tst2.js:30:9:30:9 | p | +| tst2.js:30:9:30:9 | p | +| tst2.js:33:11:33:11 | p | +| tst2.js:36:12:36:12 | p | +| tst2.js:36:12:36:12 | p | +| tst2.js:37:12:37:18 | other.p | +| tst2.js:37:12:37:18 | other.p | edges | ReflectedXss.js:8:33:8:45 | req.params.id | ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | | ReflectedXss.js:8:33:8:45 | req.params.id | ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | @@ -318,6 +326,13 @@ edges | tst2.js:14:7:14:24 | p | tst2.js:21:14:21:14 | p | | tst2.js:14:9:14:9 | p | tst2.js:14:7:14:24 | p | | tst2.js:14:9:14:9 | p | tst2.js:14:7:14:24 | p | +| tst2.js:30:7:30:24 | p | tst2.js:33:11:33:11 | p | +| tst2.js:30:7:30:24 | p | tst2.js:36:12:36:12 | p | +| tst2.js:30:7:30:24 | p | tst2.js:36:12:36:12 | p | +| tst2.js:30:9:30:9 | p | tst2.js:30:7:30:24 | p | +| tst2.js:30:9:30:9 | p | tst2.js:30:7:30:24 | p | +| tst2.js:33:11:33:11 | p | tst2.js:37:12:37:18 | other.p | +| tst2.js:33:11:33:11 | p | tst2.js:37:12:37:18 | other.p | #select | ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | ReflectedXss.js:8:33:8:45 | req.params.id | ReflectedXss.js:8:14:8:45 | "Unknow ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:8:33:8:45 | req.params.id | user-provided value | | ReflectedXss.js:17:12:17:39 | "Unknow ... rams.id | ReflectedXss.js:17:31:17:39 | params.id | ReflectedXss.js:17:12:17:39 | "Unknow ... rams.id | Cross-site scripting vulnerability due to $@. | ReflectedXss.js:17:31:17:39 | params.id | user-provided value | @@ -359,3 +374,5 @@ edges | tst2.js:8:12:8:12 | r | tst2.js:6:12:6:15 | q: r | tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value | | tst2.js:18:12:18:12 | p | tst2.js:14:9:14:9 | p | tst2.js:18:12:18:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:14:9:14:9 | p | user-provided value | | tst2.js:21:14:21:14 | p | tst2.js:14:9:14:9 | p | tst2.js:21:14:21:14 | p | Cross-site scripting vulnerability due to $@. | tst2.js:14:9:14:9 | p | user-provided value | +| tst2.js:36:12:36:12 | p | tst2.js:30:9:30:9 | p | tst2.js:36:12:36:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:30:9:30:9 | p | user-provided value | +| tst2.js:37:12:37:18 | other.p | tst2.js:30:9:30:9 | p | tst2.js:37:12:37:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:30:9:30:9 | p | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected index 719a82171a8..8ddc55dde36 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected @@ -37,3 +37,5 @@ | tst2.js:8:12:8:12 | r | Cross-site scripting vulnerability due to $@. | tst2.js:6:12:6:15 | q: r | user-provided value | | tst2.js:18:12:18:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:14:9:14:9 | p | user-provided value | | tst2.js:21:14:21:14 | p | Cross-site scripting vulnerability due to $@. | tst2.js:14:9:14:9 | p | user-provided value | +| tst2.js:36:12:36:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:30:9:30:9 | p | user-provided value | +| tst2.js:37:12:37:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:30:9:30:9 | p | user-provided value | diff --git a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js index 521b6b20a7c..034b0791217 100644 --- a/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js +++ b/javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js @@ -22,3 +22,17 @@ app.get('/bar', function(req, res) { else res.send(p); // OK }); + + +const clone = require('clone'); + +app.get('/baz', function(req, res) { + let { p } = req.params; + + var obj = {}; + obj.p = p; + var other = clone(obj); + + res.send(p); // NOT OK + res.send(other.p); // NOT OK +}); \ No newline at end of file