JS: Allow comma-separated indices in Argument[..] and Parameter[..]

Author: asgerf

javascript/ql/lib/semmle/javascript/frameworks/data/internal/Shared.qll

@@ -24,8 +24,8 @@
  * 3. The `path` column API-graph-like edges to follow starting at the node selected by `package` and `type`.
  *    It is a `.`-separated list of tokens of form:
  *     - Member[x] : a property named `x`. May be a comma-separated list of named.
- *     - Argument[n]: the n-th argument to a call. May be a range of form `x-y` (inclusive).
- *     - Parameter[n]: the n-th parameter of a callback. May be a range of form `x-y` (inclusive).
+ *     - Argument[n]: the n-th argument to a call. May be a range of form `x-y` (inclusive) and/or a comma-separated list.
+ *     - Parameter[n]: the n-th parameter of a callback. May be a range of form `x-y` (inclusive) and/or a comma-separated list.
  *     - ReturnValue: the value returned by a function call
  *     - Instance: the value returned by a constructor call
  *     - Awaited: the value from a resolved promise/future-like object
@@ -383,20 +383,20 @@ private string getApiGraphLabelFromPathToken(string token) {
       // use-node represents be an argument, and an edge originating from a def-node represents a parameter.
       // We just map both to the same thing.
       token = ["Argument[" + arg + "]", "Parameter[" + arg + "]"] and
-      result = API::EdgeLabel::parameterByStringIndex(arg)
+      exists(string part | part = arg.splitAt(",") |
+        result = API::EdgeLabel::parameterByStringIndex(part)
+        or
+        exists(string lo, string hi |
+          lo = part.regexpCapture("(\\d+)-(\\d+)", 1) and
+          hi = part.regexpCapture("(\\d+)-(\\d+)", 2) and
+          result = API::EdgeLabel::parameter([lo.toInt() .. hi.toInt()])
+        )
+      )
       or
       token = "Member[" + arg + "]" and
       result = API::EdgeLabel::member(arg.splitAt(","))
     )
     or
-    exists(string lo, string hi, string regexp |
-      // For tokens like Argument[1-5] we just enumerate the whole range of corresponding edge labels
-      regexp = "(?:Argument|Parameter)\\[(\\d+)-(\\d+)\\]" and
-      lo = token.regexpCapture(regexp, 1) and
-      hi = token.regexpCapture(regexp, 2) and
-      result = API::EdgeLabel::parameter([lo.toInt() .. hi.toInt()])
-    )
-    or
     token = "ReturnValue" and
     result = API::EdgeLabel::return()
     or

javascript/ql/test/library-tests/frameworks/data/test.expected

@@ -5,3 +5,5 @@ taintFlow
 | test.js:7:41:7:48 | source() | test.js:7:8:7:49 | require ... urce()) |
 | test.js:13:29:13:36 | source() | test.js:14:10:14:10 | y |
 | test.js:19:29:19:36 | source() | test.js:20:10:20:10 | y |
+| test.js:28:38:28:45 | source() | test.js:28:8:28:55 | testlib ... , 1, 1) |
+| test.js:30:44:30:51 | source() | test.js:30:8:30:55 | testlib ... e(), 1) |

javascript/ql/test/library-tests/frameworks/data/test.js

@@ -23,3 +23,10 @@ function testTaintIntoCallback(x) {
     sink(y); // OK - only callback 1-2 receive taint
   });
 }
+
+function testPreserveArgZeroAndTwo() {
+  sink(testlib.preserveArgZeroAndTwo(source(), 1, 1, 1)); // NOT OK
+  sink(testlib.preserveArgZeroAndTwo(1, source(), 1, 1)); // OK
+  sink(testlib.preserveArgZeroAndTwo(1, 1, source(), 1)); // NOT OK
+  sink(testlib.preserveArgZeroAndTwo(1, 1, 1, source())); // OK
+}

javascript/ql/test/library-tests/frameworks/data/test.ql

@@ -7,7 +7,8 @@ class Steps extends ModelInput::SummaryModelCsv {
     row =
       [
         "testlib;;Member[preserveTaint];Argument[0];ReturnValue;taint",
-        "testlib;;Member[taintIntoCallback];Argument[0];Argument[1-2].Parameter[0];taint"
+        "testlib;;Member[taintIntoCallback];Argument[0];Argument[1-2].Parameter[0];taint",
+        "testlib;;Member[preserveArgZeroAndTwo];Argument[0,2];ReturnValue;taint",
       ]
   }
 }