recognize object transformations in module.exports when looking for library inputs

Author: erik-krogh

javascript/ql/src/semmle/javascript/PackageExports.qll

@@ -42,7 +42,7 @@ PackageJSON getTopmostPackageJSON() {
  * Gets a value exported by the main module from one of the topmost `package.json` files (see `getTopmostPackageJSON`).
  * The value is either directly the `module.exports` value, a nested property of `module.exports`, or a method on an exported class.
  */
-private DataFlow::Node getAValueExportedByPackage() {
+DataFlow::Node getAValueExportedByPackage() {
   result = getAnExportFromModule(getTopmostPackageJSON().getMainModule())
   or
   result = getAValueExportedByPackage().(DataFlow::PropWrite).getRhs()
@@ -70,6 +70,61 @@ private DataFlow::Node getAValueExportedByPackage() {
     result = cla.getAStaticMethod() or
     result = cla.getConstructor()
   )
+  or
+  // *****
+  // Various standard library methods for transforming exported objects.
+  // *****
+  //
+  // Object.defineProperties
+  exists(DataFlow::MethodCallNode call |
+    call = DataFlow::globalVarRef("Object").getAMethodCall("defineProperties") and
+    [call, call.getArgument(0)] = getAValueExportedByPackage() and
+    result = call.getArgument(any(int i | i > 0))
+  )
+  or
+  // Object.defineProperty
+  exists(DataFlow::MethodCallNode call |
+    call = DataFlow::globalVarRef("Object").getAMethodCall("defineProperty") and
+    [call, call.getArgument(0)] = getAValueExportedByPackage()
+  |
+    result = call.getArgument(2).getALocalSource().getAPropertyReference("value")
+    or
+    result =
+      call.getArgument(2)
+          .getALocalSource()
+          .getAPropertyReference("get")
+          .(DataFlow::FunctionNode)
+          .getAReturn()
+  )
+  or
+  // Object.assign
+  exists(DataFlow::MethodCallNode assign |
+    assign = DataFlow::globalVarRef("Object").getAMethodCall("assign")
+  |
+    getAValueExportedByPackage() = [assign, assign.getArgument(0)] and
+    result = assign.getAnArgument()
+  )
+  or
+  // Array.prototype.{map, reduce, entries, values}
+  exists(DataFlow::MethodCallNode map |
+    map.getMethodName() = ["map", "reduce", "entries", "values"] and
+    map = getAValueExportedByPackage()
+  |
+    result = map.getArgument(0).getABoundFunctionValue(_).getAReturn()
+    or
+    // assuming that the receiver of the call is somehow exported
+    result = map.getReceiver()
+  )
+  or
+  // Object.{fromEntries, freeze, entries, values}
+  exists(DataFlow::MethodCallNode freeze |
+    freeze =
+      DataFlow::globalVarRef("Object")
+          .getAMethodCall(["fromEntries", "freeze", "entries", "values"])
+  |
+    freeze = getAValueExportedByPackage() and
+    result = freeze.getArgument(0)
+  )
 }
 
 /**

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

@@ -252,6 +252,12 @@ class TypeBackTracker extends TTypeBackTracker {
    */
   predicate start() { hasReturn = false and prop = "" }
 
+  /**
+   * Holds if this is the starting point of type backtracking, and the value is in the property named `propName`.
+   * The type tracking only ends after the property has been stored.
+   */
+  predicate isInProp(PropertyName propName) { hasReturn = false and prop = propName }
+
   /**
    * Holds if this is the end point of type tracking.
    */

javascript/ql/test/query-tests/Security/CWE-078/UnsafeShellCommandConstruction.expected

@@ -209,6 +209,10 @@ nodes
 | lib/lib.js:413:39:413:42 | name |
 | lib/lib.js:414:24:414:27 | name |
 | lib/lib.js:414:24:414:27 | name |
+| lib/lib.js:418:20:418:23 | name |
+| lib/lib.js:418:20:418:23 | name |
+| lib/lib.js:419:25:419:28 | name |
+| lib/lib.js:419:25:419:28 | name |
 edges
 | lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
 | lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name |
@@ -452,6 +456,10 @@ edges
 | lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
 | lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
 | lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name |
+| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
+| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
+| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
+| lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name |
 #select
 | lib/lib2.js:4:10:4:25 | "rm -rf " + name | lib/lib2.js:3:28:3:31 | name | lib/lib2.js:4:22:4:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:4:10:4:25 | "rm -rf " + name | String concatenation | lib/lib2.js:4:2:4:26 | cp.exec ... + name) | shell command |
 | lib/lib2.js:8:10:8:25 | "rm -rf " + name | lib/lib2.js:7:32:7:35 | name | lib/lib2.js:8:22:8:25 | name | $@ based on library input is later used in $@. | lib/lib2.js:8:10:8:25 | "rm -rf " + name | String concatenation | lib/lib2.js:8:2:8:26 | cp.exec ... + name) | shell command |
@@ -511,3 +519,4 @@ edges
 | lib/lib.js:366:17:366:56 | "learn  ... + model | lib/lib.js:360:20:360:23 | opts | lib/lib.js:366:28:366:42 | this.learn_args | $@ based on library input is later used in $@. | lib/lib.js:366:17:366:56 | "learn  ... + model | String concatenation | lib/lib.js:367:3:367:18 | cp.exec(command) | shell command |
 | lib/lib.js:406:10:406:25 | "rm -rf " + name | lib/lib.js:405:39:405:42 | name | lib/lib.js:406:22:406:25 | name | $@ based on library input is later used in $@. | lib/lib.js:406:10:406:25 | "rm -rf " + name | String concatenation | lib/lib.js:406:2:406:26 | cp.exec ... + name) | shell command |
 | lib/lib.js:414:12:414:27 | "rm -rf " + name | lib/lib.js:413:39:413:42 | name | lib/lib.js:414:24:414:27 | name | $@ based on library input is later used in $@. | lib/lib.js:414:12:414:27 | "rm -rf " + name | String concatenation | lib/lib.js:414:2:414:28 | asyncEx ... + name) | shell command |
+| lib/lib.js:419:13:419:28 | "rm -rf " + name | lib/lib.js:418:20:418:23 | name | lib/lib.js:419:25:419:28 | name | $@ based on library input is later used in $@. | lib/lib.js:419:13:419:28 | "rm -rf " + name | String concatenation | lib/lib.js:419:3:419:29 | asyncEx ... + name) | shell command |

javascript/ql/test/query-tests/Security/CWE-078/lib/lib.js

@@ -412,4 +412,31 @@ module.exports.sanitizer3 = function (name) {
 var asyncExec = require("async-execute");
 module.exports.asyncStuff = function (name) {
 	asyncExec("rm -rf " + name); // NOT OK
-}
+}
+
+const myFuncs = {
+	myFunc: function (name) {
+		asyncExec("rm -rf " + name); // NOT OK
+	}
+};
+
+module.exports.blabity = {};
+
+Object.defineProperties(
+	module.exports.blabity,
+	Object.assign(
+		{},
+		Object.entries(myFuncs).reduce(
+			(props, [ key, value ]) => Object.assign(
+				props,
+				{
+					[key]: {
+						value,
+						configurable: true,
+					},
+				},
+			),
+			{}
+		)
+	)
+);