JS: add enumeration taint flow to Remote Property Injection query

Author: Napalys

javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll

@@ -10,6 +10,7 @@
 
 import javascript
 import RemotePropertyInjectionCustomizations::RemotePropertyInjection
+private import semmle.javascript.DynamicPropertyAccess
 
 /**
  * A taint-tracking configuration for reasoning about remote property injection.
@@ -24,6 +25,10 @@ module RemotePropertyInjectionConfig implements DataFlow::ConfigSig {
     node = StringConcatenation::getRoot(any(ConstantString str).flow())
   }
 
+  predicate isAdditionalFlowStep(DataFlow::Node node1, DataFlow::Node node2) {
+    node1 = node2.(EnumeratedPropName).getSourceObject()
+  }
+
   predicate observeDiffInformedIncrementalMode() { any() }
 }
 

javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected

@@ -3,6 +3,7 @@
 | tst.js:13:15:13:18 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:13:15:13:18 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
 | tst.js:14:31:14:34 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:14:31:14:34 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
 | tst.js:16:10:16:13 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:16:10:16:13 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value |
+| tst.js:22:10:22:12 | key | tst.js:20:14:20:21 | req.body | tst.js:22:10:22:12 | key | A property name to write to depends on a $@. | tst.js:20:14:20:21 | req.body | user-provided value |
 | tstNonExpr.js:8:17:8:23 | userVal | tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:8:17:8:23 | userVal | A header name depends on a $@. | tstNonExpr.js:5:17:5:23 | req.url | user-provided value |
 edges
 | tst.js:8:6:8:52 | prop | tst.js:9:8:9:11 | prop | provenance |  |
@@ -12,6 +13,8 @@ edges
 | tst.js:8:13:8:52 | myCoolL ... rolled) | tst.js:8:6:8:52 | prop | provenance |  |
 | tst.js:8:28:8:51 | req.que ... trolled | tst.js:8:13:8:52 | myCoolL ... rolled) | provenance |  |
 | tst.js:8:28:8:51 | req.que ... trolled | tst.js:27:25:27:25 | x | provenance |  |
+| tst.js:20:14:20:21 | req.body | tst.js:21:3:21:5 | key | provenance | Config |
+| tst.js:21:3:21:5 | key | tst.js:22:10:22:12 | key | provenance |  |
 | tst.js:27:25:27:25 | x | tst.js:28:15:28:15 | x | provenance |  |
 | tst.js:28:6:28:15 | result | tst.js:29:9:29:14 | result | provenance |  |
 | tst.js:28:15:28:15 | x | tst.js:28:6:28:15 | result | provenance |  |
@@ -26,6 +29,9 @@ nodes
 | tst.js:13:15:13:18 | prop | semmle.label | prop |
 | tst.js:14:31:14:34 | prop | semmle.label | prop |
 | tst.js:16:10:16:13 | prop | semmle.label | prop |
+| tst.js:20:14:20:21 | req.body | semmle.label | req.body |
+| tst.js:21:3:21:5 | key | semmle.label | key |
+| tst.js:22:10:22:12 | key | semmle.label | key |
 | tst.js:27:25:27:25 | x | semmle.label | x |
 | tst.js:28:6:28:15 | result | semmle.label | result |
 | tst.js:28:15:28:15 | x | semmle.label | x |

javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js

@@ -17,9 +17,9 @@ app.get('/user/:id', function(req, res) {
 	res.set(headers);
 	myCoolLocalFct[req.query.x](); // OK - flagged by method name injection
 
-	Object.keys(req.body).forEach( // $ MISSING: Source
+	Object.keys(req.body).forEach( // $ Source
 		key => {
-			myObj[key] = 42; // $ MISSING: Alert
+			myObj[key] = 42; // $ Alert
 		}
 	);
 });