Merge pull request #10206 from asgerf/js/js-mad-changes

JS: Some JS-specific MaD changes

Author: asgerf

javascript/ql/lib/semmle/javascript/ApiGraphs.qll

@@ -533,8 +533,9 @@ module API {
 
   /** Gets a node corresponding to an import of module `m`. */
   Node moduleImport(string m) {
-    result = Impl::MkModuleImport(m) or
-    result = Impl::MkModuleImport(m).(Node).getMember("default")
+    result = Internal::getAModuleImportRaw(m)
+    or
+    result = ModelOutput::getATypeNode(m, "")
   }
 
   /** Gets a node corresponding to an export of module `m`. */
@@ -544,6 +545,22 @@ module API {
   module Node {
     /** Gets a node whose type has the given qualified name. */
     Node ofType(string moduleName, string exportedName) {
+      result = Internal::getANodeOfTypeRaw(moduleName, exportedName)
+      or
+      result = ModelOutput::getATypeNode(moduleName, exportedName)
+    }
+  }
+
+  /** Provides access to API graph nodes without taking into account types from models. */
+  module Internal {
+    /** Gets a node corresponding to an import of module `m` without taking into account types from models. */
+    Node getAModuleImportRaw(string m) {
+      result = Impl::MkModuleImport(m) or
+      result = Impl::MkModuleImport(m).(Node).getMember("default")
+    }
+
+    /** Gets a node whose type has the given qualified name, not including types from models. */
+    Node getANodeOfTypeRaw(string moduleName, string exportedName) {
       result = Impl::MkTypeUse(moduleName, exportedName).(Node).getInstance()
     }
   }

javascript/ql/lib/semmle/javascript/Classes.qll

@@ -493,6 +493,9 @@ class MemberDeclaration extends @property, Documentable {
    */
   predicate isStatic() { is_static(this) }
 
+  /** Gets a boolean indicating if this member is static. */
+  boolean getStaticAsBool() { if this.isStatic() then result = true else result = false }
+
   /**
    * Holds if this member is abstract.
    *
@@ -694,10 +697,10 @@ class MethodDeclaration extends MemberDeclaration {
    * the overload index is defined as if only one of them was concrete.
    */
   int getOverloadIndex() {
-    exists(ClassOrInterface type, string name |
+    exists(ClassOrInterface type, string name, boolean static |
       this =
         rank[result + 1](MethodDeclaration method, int i |
-          methodDeclaredInType(type, name, i, method)
+          methodDeclaredInType(type, name, static, i, method)
         |
           method order by i
         )
@@ -718,10 +721,11 @@ class MethodDeclaration extends MemberDeclaration {
  * Holds if the `index`th member of `type` is `method`, which has the given `name`.
  */
 private predicate methodDeclaredInType(
-  ClassOrInterface type, string name, int index, MethodDeclaration method
+  ClassOrInterface type, string name, boolean static, int index, MethodDeclaration method
 ) {
   not method instanceof ConstructorDeclaration and // distinguish methods named "constructor" from the constructor
   type.getMemberByIndex(index) = method and
+  static = method.getStaticAsBool() and
   method.getName() = name
 }
 

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

@@ -104,6 +104,9 @@ API::Node getExtraSuccessorFromNode(API::Node node, AccessPathToken token) {
   token.getName() = "Member" and
   result = node.getMember(token.getAnArgument())
   or
+  token.getName() = "AnyMember" and
+  result = node.getAMember()
+  or
   token.getName() = "Instance" and
   result = node.getInstance()
   or
@@ -163,6 +166,16 @@ predicate invocationMatchesExtraCallSiteFilter(API::InvokeNode invoke, AccessPat
   token.getName() = "Call" and
   invoke instanceof API::CallNode and
   invoke instanceof DataFlow::CallNode // Workaround compiler bug
+  or
+  token.getName() = "WithStringArgument" and
+  exists(string operand, string argIndex, string stringValue |
+    operand = token.getAnArgument() and
+    argIndex = operand.splitAt("=", 0) and
+    stringValue = operand.splitAt("=", 1) and
+    invoke
+        .getArgument(AccessPath::parseIntWithArity(argIndex, invoke.getNumArgument()))
+        .getStringValue() = stringValue
+  )
 }
 
 /**
@@ -225,8 +238,9 @@ bindingset[name]
 predicate isExtraValidTokenNameInIdentifyingAccessPath(string name) {
   name =
     [
-      "Member", "Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call",
-      "DecoratedClass", "DecoratedMember", "DecoratedParameter"
+      "Member", "AnyMember", "Instance", "Awaited", "ArrayElement", "Element", "MapValue",
+      "NewCall", "Call", "DecoratedClass", "DecoratedMember", "DecoratedParameter",
+      "WithStringArgument"
     ]
 }
 
@@ -237,7 +251,7 @@ predicate isExtraValidTokenNameInIdentifyingAccessPath(string name) {
 predicate isExtraValidNoArgumentTokenInIdentifyingAccessPath(string name) {
   name =
     [
-      "Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call",
+      "AnyMember", "Instance", "Awaited", "ArrayElement", "Element", "MapValue", "NewCall", "Call",
       "DecoratedClass", "DecoratedMember", "DecoratedParameter"
     ]
 }
@@ -250,4 +264,8 @@ bindingset[name, argument]
 predicate isExtraValidTokenArgumentInIdentifyingAccessPath(string name, string argument) {
   name = ["Member"] and
   exists(argument)
+  or
+  name = "WithStringArgument" and
+  exists(argument.indexOf("=")) and
+  exists(AccessPath::parseIntWithArity(argument.splitAt("=", 0), 10))
 }

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

@@ -55,6 +55,9 @@ taintFlow
 | test.js:182:12:182:19 | source() | test.js:182:12:182:19 | source() |
 | test.js:187:31:187:31 | x | test.js:189:10:189:10 | x |
 | test.js:203:32:203:39 | source() | test.js:203:32:203:39 | source() |
+| 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() |
 isSink
 | test.js:54:18:54:25 | source() | test-sink |
 | test.js:55:22:55:29 | source() | test-sink |
@@ -113,6 +116,9 @@ isSink
 | test.js:182:12:182:19 | source() | test-sink |
 | test.js:196:12:196:29 | this._wrappedField | test-sink |
 | test.js:203:32:203:39 | source() | test-sink |
+| 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 |
 syntaxErrors
 | Member[foo |
 | Member[foo] .Member[bar] |

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

@@ -203,3 +203,9 @@ class OtherClass {
     this.accessorAroundField = source(); // NOT OK
   }
 }
+
+testlib.foo.memberSink(source()); // NOT OK
+testlib.bar.memberSink(source()); // NOT OK
+testlib.memberSink(source()); // OK
+testlib.overloadedSink('safe', source()); // OK
+testlib.overloadedSink('danger', source()); // NOT OK

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

@@ -38,6 +38,8 @@ class Sinks extends ModelInput::SinkModelCsv {
         "testlib;;Member[MethodDecorator].DecoratedMember.ReturnValue;test-sink",
         "testlib;;Member[MethodDecoratorWithArgs].ReturnValue.DecoratedMember.ReturnValue;test-sink",
         "testlib;;Member[ParamDecoratorSink].DecoratedParameter;test-sink",
+        "testlib;;AnyMember.Member[memberSink].Argument[0];test-sink",
+        "testlib;;Member[overloadedSink].WithStringArgument[0=danger].Argument[1];test-sink",
       ]
   }
 }