JS: Use ValueNode, not SSA node, to model NamedImportSpecifier

Author: asger-semmle

javascript/ql/src/semmle/javascript/DefUse.qll

@@ -40,6 +40,8 @@ private predicate defn(ControlFlowNode def, Expr lhs, AST::ValueNode rhs) {
   or
   exists(ImportEqualsDeclaration i | def = i | lhs = i.getId() and rhs = i.getImportedEntity())
   or
+  exists(ImportSpecifier i | def = i | lhs = i.getLocal() and rhs = i)
+  or
   exists(EnumMember member | def = member.getIdentifier() |
     lhs = def and rhs = member.getInitializer()
   )
@@ -71,8 +73,6 @@ private predicate defn(ControlFlowNode def, Expr lhs) {
   or
   lhs = def.(UpdateExpr).getOperand().getUnderlyingReference()
   or
-  lhs = def.(ImportSpecifier).getLocal()
-  or
   exists(EnhancedForLoop efl | def = efl.getIteratorExpr() |
     lhs = def.(Expr).stripParens() or
     lhs = def.(VariableDeclarator).getBindingPattern()

javascript/ql/src/semmle/javascript/dataflow/DataFlow.qll

@@ -799,21 +799,21 @@ module DataFlow {
   /**
    * A named import specifier seen as a property read on the imported module.
    */
-  private class ImportSpecifierAsPropRead extends PropRead {
+  private class ImportSpecifierAsPropRead extends PropRead, ValueNode {
+    override ImportSpecifier astNode;
+
     ImportDeclaration imprt;
-    ImportSpecifier spec;
 
     ImportSpecifierAsPropRead() {
-      spec = imprt.getASpecifier() and
-      exists(spec.getImportedName()) and
-      this = ssaDefinitionNode(SSA::definition(spec))
+      astNode = imprt.getASpecifier() and
+      exists(astNode.getImportedName())
     }
 
     override Node getBase() { result = TDestructuredModuleImportNode(imprt) }
 
-    override Expr getPropertyNameExpr() { result = spec.getImported() }
+    override Expr getPropertyNameExpr() { result = astNode.getImported() }
 
-    override string getPropertyName() { result = spec.getImportedName() }
+    override string getPropertyName() { result = astNode.getImportedName() }
   }
 
   /**

javascript/ql/src/semmle/javascript/dataflow/Sources.qll

@@ -234,10 +234,13 @@ module SourceNode {
         astNode instanceof AwaitExpr or
         astNode instanceof FunctionSentExpr or
         astNode instanceof FunctionBindExpr or
-        astNode instanceof DynamicImportExpr
+        astNode instanceof DynamicImportExpr or
+        astNode instanceof NamedImportSpecifier
       )
       or
-      this = DataFlow::ssaDefinitionNode(SSA::definition(any(ImportSpecifier imp)))
+      this = DataFlow::ssaDefinitionNode(SSA::definition(any(ImportNamespaceSpecifier imp)))
+      or
+      this = DataFlow::ssaDefinitionNode(SSA::definition(any(ImportDefaultSpecifier imp)))
       or
       DataFlow::parameterNode(this, _)
       or

javascript/ql/test/library-tests/DataFlow/flowStep.expected

@@ -9,6 +9,7 @@
 | sources.js:5:4:5:5 | 23 | sources.js:3:11:3:11 | x |
 | tst.js:1:1:1:1 | x | tst.js:28:2:28:1 | x |
 | tst.js:1:1:1:1 | x | tst.js:32:1:32:0 | x |
+| tst.js:1:10:1:11 | fs | tst.js:1:10:1:11 | fs |
 | tst.js:1:10:1:11 | fs | tst.js:7:1:7:2 | fs |
 | tst.js:1:10:1:11 | fs | tst.js:22:24:22:25 | fs |
 | tst.js:3:5:3:10 | x | tst.js:8:1:8:1 | x |