JavaScript: Refactor AnalyzedPropertyWrite::writes to enable correct modelling of variable exports.

(cherry picked from commit 080f974663)
2026-05-01 03:35:13 +02:00 · 2018-10-09 12:55:38 +01:00
parent 898ba94837
commit 5167d43fbc
8 changed files with 40 additions and 10 deletions
--- a/javascript/ql/src/semmle/javascript/dataflow/AbstractProperties.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/AbstractProperties.qll
@@ -96,8 +96,7 @@ class AbstractProtoProperty extends AbstractProperty {
 * which in turn introduces a materialization.
 */
 private AbstractValue getAnAssignedValue(AbstractValue b, string p) {
-  exists (AnalyzedPropertyWrite apw, DataFlow::AnalyzedNode afn |
-    apw.writes(b, p, afn) and
-    result = afn.getALocalValue()
+  exists (AnalyzedPropertyWrite apw |
+    apw.writesValue(b, p, result)
  )
 }
--- a/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/InterModuleTypeInference.qll
@@ -263,10 +263,10 @@ private class AnalyzedVariableExport extends AnalyzedPropertyWrite, DataFlow::Va
    astNode = varDef.getTarget()
  }

-  override predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
+  override predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
    baseVal = TAbstractExportsObject(export.getEnclosingModule()) and
    propName = name and
-    source = varDef.getSource().analyze()
+    val = varDef.getAnAssignedValue()
  }
 }

--- a/javascript/ql/src/semmle/javascript/dataflow/internal/PropertyTypeInference.qll
+++ b/javascript/ql/src/semmle/javascript/dataflow/internal/PropertyTypeInference.qll
@@ -96,8 +96,24 @@ abstract class AnalyzedPropertyWrite extends DataFlow::Node {
  /**
   * Holds if this property write assigns `source` to property `propName` of one of the
   * concrete objects represented by `baseVal`.
+   *
+   * Note that not all property writes have an explicit `source` node; use predicate
+   * `writesValue` below to cover these cases.
   */
-  abstract predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source);
+  predicate writes(AbstractValue baseVal, string propName, DataFlow::AnalyzedNode source) {
+    none()
+  }
+
+  /**
+   * Holds if this property write assigns `val` to property `propName` of one of the
+   * concrete objects represented by `baseVal`.
+   */
+  predicate writesValue(AbstractValue baseVal, string propName, AbstractValue val) {
+    exists (AnalyzedNode source |
+      writes(baseVal, propName, source) and
+      val = source.getALocalValue()
+    )
+  }

  /**
   * Holds if the flow information for the base node of this property write is incomplete
--- a/javascript/ql/test/library-tests/Flow/AbstractValues.expected
+++ b/javascript/ql/test/library-tests/Flow/AbstractValues.expected
@@ -4,10 +4,12 @@
 | ChatListScreen.js:3:1:5:1 | instance of function foo |
 | a2.js:1:1:2:0 | exports object of module a2 |
 | a2.js:1:1:2:0 | module object of module a2 |
-| a.js:1:1:13:0 | exports object of module a |
-| a.js:1:1:13:0 | module object of module a |
+| a.js:1:1:18:0 | exports object of module a |
+| a.js:1:1:18:0 | module object of module a |
 | a.js:3:8:5:1 | function setX |
 | a.js:3:8:5:1 | instance of function setX |
+| a.js:15:1:17:1 | function bump |
+| a.js:15:1:17:1 | instance of function bump |
 | amd2.js:1:1:4:0 | exports object of module amd2 |
 | amd2.js:1:1:4:0 | module object of module amd2 |
 | amd2.js:1:8:3:1 | anonymous function |
@@ -36,8 +38,8 @@
 | arguments.js:30:2:33:1 | anonymous function |
 | arguments.js:30:2:33:1 | arguments object of anonymous function |
 | arguments.js:30:2:33:1 | instance of anonymous function |
-| b.js:1:1:55:0 | exports object of module b |
-| b.js:1:1:55:0 | module object of module b |
+| b.js:1:1:58:0 | exports object of module b |
+| b.js:1:1:58:0 | module object of module b |
 | backend.js:1:1:3:0 | exports object of module backend |
 | backend.js:1:1:3:0 | module object of module backend |
 | backend.js:1:17:1:18 | object literal |
--- a/javascript/ql/test/library-tests/Flow/a.js
+++ b/javascript/ql/test/library-tests/Flow/a.js
@@ -10,3 +10,8 @@ let z = someGlobal;

 export let w;
 w = "w";
+
+export let notAlwaysZero = 0;
+function bump() {
+  ++notAlwaysZero;
+}
--- a/javascript/ql/test/library-tests/Flow/abseval.expected
+++ b/javascript/ql/test/library-tests/Flow/abseval.expected
@@ -6,6 +6,7 @@
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | indefinite value (global) |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | non-zero value |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | file://:0:0:0:0 | true |
+| a.js:14:12:14:24 | notAlwaysZero | a.js:14:28:14:28 | 0 | file://:0:0:0:0 | 0 |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | amd.js:1:1:7:0 | module object of module amd |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | file://:0:0:0:0 | indefinite value (call) |
 | amd.js:3:7:3:7 | e | amd.js:3:11:3:13 | exp | amd.js:1:1:7:0 | exports object of module amd |
@@ -58,6 +59,8 @@
 | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | file://:0:0:0:0 | non-empty, non-numeric string |
 | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | file://:0:0:0:0 | indefinite value (import) |
 | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | file://:0:0:0:0 | indefinite value (import) |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | 0 |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | file://:0:0:0:0 | non-zero value |
 | backend.js:1:7:1:13 | Backend | backend.js:1:17:1:18 | {} | backend.js:1:17:1:18 | object literal |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (call) |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | file://:0:0:0:0 | indefinite value (heap) |
--- a/javascript/ql/test/library-tests/Flow/b.js
+++ b/javascript/ql/test/library-tests/Flow/b.js
@@ -52,3 +52,6 @@ let z14 = foo_reexported;

 import { something } from './reexport-unknown';
 let z15 = something;
+
+import { notAlwaysZero } from './a';
+let z16 = notAlwaysZero;
--- a/javascript/ql/test/library-tests/Flow/types.expected
+++ b/javascript/ql/test/library-tests/Flow/types.expected
@@ -2,6 +2,7 @@
 | a.js:1:12:1:12 | x | a.js:1:16:1:16 | 0 | number |
 | a.js:1:19:1:19 | y | a.js:1:23:1:23 | 0 | number |
 | a.js:9:5:9:5 | z | a.js:9:9:9:18 | someGlobal | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| a.js:14:12:14:24 | notAlwaysZero | a.js:14:28:14:28 | 0 | number |
 | amd.js:2:7:2:7 | m | amd.js:2:11:2:13 | mod | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | amd.js:3:7:3:7 | e | amd.js:3:11:3:13 | exp | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | arguments.js:2:7:2:7 | y | arguments.js:2:11:2:11 | x | number |
@@ -32,6 +33,7 @@
 | b.js:48:5:48:7 | z13 | b.js:48:11:48:11 | w | string |
 | b.js:51:5:51:7 | z14 | b.js:51:11:51:24 | foo_reexported | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | b.js:54:5:54:7 | z15 | b.js:54:11:54:19 | something | boolean, class, date, function, null, number, object, regular expression,string or undefined |
+| b.js:57:5:57:7 | z16 | b.js:57:11:57:23 | notAlwaysZero | number |
 | backend.js:1:7:1:13 | Backend | backend.js:1:17:1:18 | {} | object |
 | classAccessors.js:10:9:10:11 | myX | classAccessors.js:10:15:10:20 | this.x | boolean, class, date, function, null, number, object, regular expression,string or undefined |
 | classAccessors.js:11:9:11:11 | myY | classAccessors.js:11:15:11:20 | this.y | boolean, class, date, function, null, number, object, regular expression,string or undefined |