Merge pull request #10205 from asgerf/mad-generics

Support type variables in MaD typings

Author: asgerf

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

@@ -155,6 +155,22 @@ module ModelInput {
      */
     abstract predicate row(string row);
   }
+
+  /**
+   * A unit class for adding additional type variable model rows.
+   */
+  class TypeVariableModelCsv extends Unit {
+    /**
+     * Holds if `row` specifies a path through a type variable.
+     *
+     * A row of form,
+     * ```
+     * name;path
+     * ```
+     * means `path` can be substituted for a token `TypeVar[name]`.
+     */
+    abstract predicate row(string row);
+  }
 }
 
 private import ModelInput
@@ -182,6 +198,8 @@ private predicate summaryModel(string row) { any(SummaryModelCsv s).row(inverseP
 
 private predicate typeModel(string row) { any(TypeModelCsv s).row(inversePad(row)) }
 
+private predicate typeVariableModel(string row) { any(TypeVariableModelCsv s).row(inversePad(row)) }
+
 /** Holds if a source model exists for the given parameters. */
 predicate sourceModel(string package, string type, string path, string kind) {
   exists(string row |
@@ -219,7 +237,7 @@ private predicate summaryModel(
   )
 }
 
-/** Holds if an type model exists for the given parameters. */
+/** Holds if a type model exists for the given parameters. */
 private predicate typeModel(
   string package1, string type1, string package2, string type2, string path
 ) {
@@ -233,6 +251,15 @@ private predicate typeModel(
   )
 }
 
+/** Holds if a type variable model exists for the given parameters. */
+private predicate typeVariableModel(string name, string path) {
+  exists(string row |
+    typeVariableModel(row) and
+    row.splitAt(";", 0) = name and
+    row.splitAt(";", 1) = path
+  )
+}
+
 /**
  * Gets a package that should be seen as an alias for the given other `package`,
  * or the `package` itself.
@@ -253,7 +280,7 @@ private predicate isRelevantPackage(string package) {
     sourceModel(package, _, _, _) or
     sinkModel(package, _, _, _) or
     summaryModel(package, _, _, _, _, _) or
-    typeModel(package, _, _, _, _)
+    typeModel(_, _, package, _, _)
   ) and
   (
     Specific::isPackageUsed(package)
@@ -290,6 +317,8 @@ private class AccessPathRange extends AccessPath::Range {
       summaryModel(package, _, _, this, _, _) or
       summaryModel(package, _, _, _, this, _)
     )
+    or
+    typeVariableModel(_, this)
   }
 }
 
@@ -361,13 +390,93 @@ private API::Node getNodeFromPath(string package, string type, AccessPath path,
   // Similar to the other recursive case, but where the path may have stepped through one or more call-site filters
   result =
     getSuccessorFromInvoke(getInvocationFromPath(package, type, path, n - 1), path.getToken(n - 1))
+  or
+  // Apply a subpath
+  result =
+    getNodeFromSubPath(getNodeFromPath(package, type, path, n - 1), getSubPathAt(path, n - 1))
+  or
+  // Apply a type step
+  typeStep(getNodeFromPath(package, type, path, n), result)
+}
+
+/**
+ * Gets a subpath for the `TypeVar` token found at the `n`th token of `path`.
+ */
+pragma[nomagic]
+private AccessPath getSubPathAt(AccessPath path, int n) {
+  exists(string typeVarName |
+    path.getToken(n).getAnArgument("TypeVar") = typeVarName and
+    typeVariableModel(typeVarName, result)
+  )
+}
+
+/**
+ * Gets a node that is found by evaluating the first `n` tokens of `subPath` starting at `base`.
+ */
+pragma[nomagic]
+private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath, int n) {
+  exists(AccessPath path, int k |
+    base = [getNodeFromPath(_, _, path, k), getNodeFromSubPath(_, path, k)] and
+    subPath = getSubPathAt(path, k) and
+    result = base and
+    n = 0
+  )
+  or
+  exists(string package, string type, AccessPath basePath |
+    typeStepModel(package, type, basePath, subPath) and
+    base = getNodeFromPath(package, type, basePath) and
+    result = base and
+    n = 0
+  )
+  or
+  result = getSuccessorFromNode(getNodeFromSubPath(base, subPath, n - 1), subPath.getToken(n - 1))
+  or
+  result =
+    getSuccessorFromInvoke(getInvocationFromSubPath(base, subPath, n - 1), subPath.getToken(n - 1))
+  or
+  result =
+    getNodeFromSubPath(getNodeFromSubPath(base, subPath, n - 1), getSubPathAt(subPath, n - 1))
+  or
+  typeStep(getNodeFromSubPath(base, subPath, n), result)
+}
+
+/**
+ * Gets a call site that is found by evaluating the first `n` tokens of `subPath` starting at `base`.
+ */
+private Specific::InvokeNode getInvocationFromSubPath(API::Node base, AccessPath subPath, int n) {
+  result = Specific::getAnInvocationOf(getNodeFromSubPath(base, subPath, n))
+  or
+  result = getInvocationFromSubPath(base, subPath, n - 1) and
+  invocationMatchesCallSiteFilter(result, subPath.getToken(n - 1))
+}
+
+/**
+ * Gets a node that is found by evaluating `subPath` starting at `base`.
+ */
+pragma[nomagic]
+private API::Node getNodeFromSubPath(API::Node base, AccessPath subPath) {
+  result = getNodeFromSubPath(base, subPath, subPath.getNumToken())
 }
 
 /** Gets the node identified by the given `(package, type, path)` tuple. */
 API::Node getNodeFromPath(string package, string type, AccessPath path) {
   result = getNodeFromPath(package, type, path, path.getNumToken())
 }
 
+pragma[nomagic]
+private predicate typeStepModel(string package, string type, AccessPath basePath, AccessPath output) {
+  summaryModel(package, type, basePath, "", output, "type")
+}
+
+pragma[nomagic]
+private predicate typeStep(API::Node pred, API::Node succ) {
+  exists(string package, string type, AccessPath basePath, AccessPath output |
+    typeStepModel(package, type, basePath, output) and
+    pred = getNodeFromPath(package, type, basePath) and
+    succ = getNodeFromSubPath(pred, output)
+  )
+}
+
 /**
  * Gets an invocation identified by the given `(package, type, path)` tuple.
  *
@@ -390,7 +499,7 @@ Specific::InvokeNode getInvocationFromPath(string package, string type, AccessPa
  */
 bindingset[name]
 predicate isValidTokenNameInIdentifyingAccessPath(string name) {
-  name = ["Argument", "Parameter", "ReturnValue", "WithArity"]
+  name = ["Argument", "Parameter", "ReturnValue", "WithArity", "TypeVar"]
   or
   Specific::isExtraValidTokenNameInIdentifyingAccessPath(name)
 }
@@ -418,6 +527,9 @@ predicate isValidTokenArgumentInIdentifyingAccessPath(string name, string argume
   name = "WithArity" and
   argument.regexpMatch("\\d+(\\.\\.(\\d+)?)?")
   or
+  name = "TypeVar" and
+  exists(argument)
+  or
   Specific::isExtraValidTokenArgumentInIdentifyingAccessPath(name, argument)
 }
 
@@ -489,6 +601,8 @@ module ModelOutput {
       any(SummaryModelCsv csv).row(row) and kind = "summary" and expectedArity = 6
       or
       any(TypeModelCsv csv).row(row) and kind = "type" and expectedArity = 5
+      or
+      any(TypeVariableModelCsv csv).row(row) and kind = "type-variable" and expectedArity = 2
     |
       actualArity = count(row.indexOf(";")) + 1 and
       actualArity != expectedArity and
@@ -499,7 +613,7 @@ module ModelOutput {
     or
     // Check names and arguments of access path tokens
     exists(AccessPath path, AccessPathToken token |
-      isRelevantFullPath(_, _, path) and
+      (isRelevantFullPath(_, _, path) or typeVariableModel(_, path)) and
       token = path.getToken(_)
     |
       not isValidTokenNameInIdentifyingAccessPath(token.getName()) and

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

@@ -58,6 +58,14 @@ taintFlow
 | test.js:207:24:207:31 | source() | test.js:207:24:207:31 | source() |
 | test.js:208:24:208:31 | source() | test.js:208:24:208:31 | source() |
 | test.js:211:34:211:41 | source() | test.js:211:34:211:41 | source() |
+| test.js:214:34:214:41 | source() | test.js:214:34:214:41 | source() |
+| test.js:223:45:223:52 | source() | test.js:223:45:223:52 | source() |
+| test.js:225:39:225:46 | source() | test.js:225:39:225:46 | source() |
+| test.js:226:50:226:57 | source() | test.js:226:50:226:57 | source() |
+| test.js:230:59:230:66 | source() | test.js:230:59:230:66 | source() |
+| test.js:231:59:231:66 | source() | test.js:231:59:231:66 | source() |
+| test.js:232:59:232:66 | source() | test.js:232:59:232:66 | source() |
+| test.js:233:59:233:66 | source() | test.js:233:59:233:66 | source() |
 isSink
 | test.js:54:18:54:25 | source() | test-sink |
 | test.js:55:22:55:29 | source() | test-sink |
@@ -119,6 +127,15 @@ isSink
 | test.js:207:24:207:31 | source() | test-sink |
 | test.js:208:24:208:31 | source() | test-sink |
 | test.js:211:34:211:41 | source() | test-sink |
+| test.js:214:34:214:41 | source() | test-sink |
+| test.js:222:52:222:52 | 0 | test-sink |
+| test.js:223:45:223:52 | source() | test-sink |
+| test.js:225:39:225:46 | source() | test-sink |
+| test.js:226:50:226:57 | source() | test-sink |
+| test.js:230:59:230:66 | source() | test-sink |
+| test.js:231:59:231:66 | source() | test-sink |
+| test.js:232:59:232:66 | source() | test-sink |
+| test.js:233:59:233:66 | source() | test-sink |
 syntaxErrors
 | Member[foo |
 | Member[foo] .Member[bar] |

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

@@ -209,3 +209,26 @@ testlib.bar.memberSink(source()); // NOT OK
 testlib.memberSink(source()); // OK
 testlib.overloadedSink('safe', source()); // OK
 testlib.overloadedSink('danger', source()); // NOT OK
+
+function typeVars() {
+  testlib.typevar.a.b().c.mySink(source()); // NOT OK
+
+  testlib.typevar.mySink(source()); // OK - does not match sub path
+  testlib.typevar.a.mySink(source()); // OK - does not match sub path
+  testlib.typevar.a.b.mySink(source()); // OK - does not match sub path
+  testlib.typevar.a.b.c.mySink(source()); // OK - does not match sub path
+  testlib.typevar.a.b(1).c.mySink(source()); // OK - does not match sub path
+
+  testlib.typevar.a.b().c.a.b().c.mySink(source(), 0); // OK
+  testlib.typevar.a.b().c.a.b().c.mySink(0, source()); // NOT OK
+
+  testlib.typevar.left.x.right.mySink(source()); // NOT OK
+  testlib.typevar.left.left.x.right.right.mySink(source()); // NOT OK
+  testlib.typevar.left.x.right.right.mySink(source()); // OK - mismatched left and right
+  testlib.typevar.left.left.x.right.mySink(source()); // OK - mismatched left and right
+
+  testlib.typevar.getThis().getThis().left.x.right.mySink(source()); // NOT OK
+  testlib.typevar.left.getThis().getThis().x.right.mySink(source()); // NOT OK
+  testlib.typevar.left.x.getThis().getThis().right.mySink(source()); // NOT OK
+  testlib.typevar.left.x.right.getThis().getThis().mySink(source()); // NOT OK
+}

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

@@ -14,6 +14,17 @@ class Steps extends ModelInput::SummaryModelCsv {
         "testlib;;Member[preserveAllButFirstArgument];Argument[1..];ReturnValue;taint",
         "testlib;;Member[preserveAllIfCall].Call;Argument[0..];ReturnValue;taint",
         "testlib;;Member[getSource].ReturnValue.Member[continue];Argument[this];ReturnValue;taint",
+        "testlib;~HasThisFlow;;;Member[getThis].ReturnValue;type",
+      ]
+  }
+}
+
+class TypeDefs extends ModelInput::TypeModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "testlib;~HasThisFlow;testlib;;Member[typevar]",
+        "testlib;~HasThisFlow;testlib;~HasThisFlow;Member[left,right,x]",
       ]
   }
 }
@@ -40,6 +51,20 @@ class Sinks extends ModelInput::SinkModelCsv {
         "testlib;;Member[ParamDecoratorSink].DecoratedParameter;test-sink",
         "testlib;;AnyMember.Member[memberSink].Argument[0];test-sink",
         "testlib;;Member[overloadedSink].WithStringArgument[0=danger].Argument[1];test-sink",
+        "testlib;;Member[typevar].TypeVar[ABC].Member[mySink].Argument[0];test-sink",
+        "testlib;;Member[typevar].TypeVar[ABC].TypeVar[ABC].Member[mySink].Argument[1];test-sink",
+        "testlib;;Member[typevar].TypeVar[LeftRight].Member[mySink].Argument[0];test-sink",
+      ]
+  }
+}
+
+class TypeVars extends ModelInput::TypeVariableModelCsv {
+  override predicate row(string row) {
+    row =
+      [
+        "ABC;Member[a].Member[b].WithArity[0].ReturnValue.Member[c]", //
+        "LeftRight;Member[left].TypeVar[LeftRight].Member[right]", //
+        "LeftRight;Member[x]",
       ]
   }
 }