Merge pull request #6295 from erik-krogh/sort-keys

Approved by asgerf

Author: codeql-ci

javascript/change-notes/2021-07-15-sort-keys.md

@@ -0,0 +1,5 @@
+lgtm,codescanning
+* The dataflow libraries now model dataflow in the `sort-keys` and `camelcase-keys` library.
+  Affected packages are
+    [sort-keys](https://npmjs.com/package/sort-keys),
+    [camelcase-keys](https://npmjs.com/package/camelcase-keys)

javascript/ql/src/semmle/javascript/Extend.qll

@@ -183,7 +183,8 @@ private import semmle.javascript.dataflow.internal.PreCallGraphStep
 private class CloneStep extends PreCallGraphStep {
   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
     exists(DataFlow::CallNode call |
-      call = DataFlow::moduleImport(["clone", "fclone"]).getACall()
+      // `camelcase-keys` isn't quite a cloning library. But it's pretty close.
+      call = DataFlow::moduleImport(["clone", "fclone", "sort-keys", "camelcase-keys"]).getACall()
       or
       call = DataFlow::moduleMember("json-cycle", ["decycle", "retrocycle"]).getACall()
     |

javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXss.expected

@@ -206,6 +206,14 @@ nodes
 | tst2.js:75:12:75:12 | p |
 | tst2.js:76:12:76:18 | other.p |
 | tst2.js:76:12:76:18 | other.p |
+| tst2.js:82:7:82:24 | p |
+| tst2.js:82:9:82:9 | p |
+| tst2.js:82:9:82:9 | p |
+| tst2.js:85:11:85:11 | p |
+| tst2.js:88:12:88:12 | p |
+| tst2.js:88:12:88:12 | p |
+| tst2.js:89:12:89:18 | other.p |
+| tst2.js:89:12:89:18 | other.p |
 | tst3.js:5:7:5:24 | p |
 | tst3.js:5:9:5:9 | p |
 | tst3.js:5:9:5:9 | p |
@@ -389,6 +397,13 @@ edges
 | tst2.js:69:9:69:9 | p | tst2.js:69:7:69:24 | p |
 | tst2.js:72:11:72:11 | p | tst2.js:76:12:76:18 | other.p |
 | tst2.js:72:11:72:11 | p | tst2.js:76:12:76:18 | other.p |
+| tst2.js:82:7:82:24 | p | tst2.js:85:11:85:11 | p |
+| tst2.js:82:7:82:24 | p | tst2.js:88:12:88:12 | p |
+| tst2.js:82:7:82:24 | p | tst2.js:88:12:88:12 | p |
+| tst2.js:82:9:82:9 | p | tst2.js:82:7:82:24 | p |
+| tst2.js:82:9:82:9 | p | tst2.js:82:7:82:24 | p |
+| tst2.js:85:11:85:11 | p | tst2.js:89:12:89:18 | other.p |
+| tst2.js:85:11:85:11 | p | tst2.js:89:12:89:18 | other.p |
 | tst3.js:5:7:5:24 | p | tst3.js:6:12:6:12 | p |
 | tst3.js:5:7:5:24 | p | tst3.js:6:12:6:12 | p |
 | tst3.js:5:9:5:9 | p | tst3.js:5:7:5:24 | p |
@@ -446,5 +461,7 @@ edges
 | tst2.js:64:12:64:18 | other.p | tst2.js:57:9:57:9 | p | tst2.js:64:12:64:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
 | tst2.js:75:12:75:12 | p | tst2.js:69:9:69:9 | p | tst2.js:75:12:75:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
 | tst2.js:76:12:76:18 | other.p | tst2.js:69:9:69:9 | p | tst2.js:76:12:76:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
+| tst2.js:88:12:88:12 | p | tst2.js:82:9:82:9 | p | tst2.js:88:12:88:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:82:9:82:9 | p | user-provided value |
+| tst2.js:89:12:89:18 | other.p | tst2.js:82:9:82:9 | p | tst2.js:89:12:89:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:82:9:82:9 | p | user-provided value |
 | tst3.js:6:12:6:12 | p | tst3.js:5:9:5:9 | p | tst3.js:6:12:6:12 | p | Cross-site scripting vulnerability due to $@. | tst3.js:5:9:5:9 | p | user-provided value |
 | tst3.js:12:12:12:15 | code | tst3.js:11:32:11:39 | reg.body | tst3.js:12:12:12:15 | code | Cross-site scripting vulnerability due to $@. | tst3.js:11:32:11:39 | reg.body | user-provided value |

javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/ReflectedXssWithCustomSanitizer.expected

@@ -44,5 +44,7 @@
 | tst2.js:64:12:64:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:57:9:57:9 | p | user-provided value |
 | tst2.js:75:12:75:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
 | tst2.js:76:12:76:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:69:9:69:9 | p | user-provided value |
+| tst2.js:88:12:88:12 | p | Cross-site scripting vulnerability due to $@. | tst2.js:82:9:82:9 | p | user-provided value |
+| tst2.js:89:12:89:18 | other.p | Cross-site scripting vulnerability due to $@. | tst2.js:82:9:82:9 | p | user-provided value |
 | tst3.js:6:12:6:12 | p | Cross-site scripting vulnerability due to $@. | tst3.js:5:9:5:9 | p | user-provided value |
 | tst3.js:12:12:12:15 | code | Cross-site scripting vulnerability due to $@. | tst3.js:11:32:11:39 | reg.body | user-provided value |

javascript/ql/test/query-tests/Security/CWE-079/ReflectedXss/tst2.js

@@ -72,6 +72,19 @@ app.get('/baz', function(req, res) {
   obj.p = p;
   var other = jc.retrocycle(jc.decycle(obj));
 
+  res.send(p); // NOT OK
+  res.send(other.p); // NOT OK
+});
+
+const sortKeys = require('sort-keys');
+
+app.get('/baz', function(req, res) {
+  let { p } = req.params;
+
+  var obj = {};
+  obj.p = p;
+  var other = sortKeys(obj);
+
   res.send(p); // NOT OK
   res.send(other.p); // NOT OK
 });