From 10c10c7d30cdd8bff78677840d5accf6cfe12a47 Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 27 Aug 2025 10:17:39 +0000 Subject: [PATCH 1/4] JS: fixed typo in folder name --- .../RemotePropertyInjection.expected | 0 .../RemotePropertyInjection.qlref | 0 .../{RemovePropertyInjection => RemotePropertyInjection}/tst.js | 0 .../tstNonExpr.js | 0 4 files changed, 0 insertions(+), 0 deletions(-) rename javascript/ql/test/query-tests/Security/CWE-400/{RemovePropertyInjection => RemotePropertyInjection}/RemotePropertyInjection.expected (100%) rename javascript/ql/test/query-tests/Security/CWE-400/{RemovePropertyInjection => RemotePropertyInjection}/RemotePropertyInjection.qlref (100%) rename javascript/ql/test/query-tests/Security/CWE-400/{RemovePropertyInjection => RemotePropertyInjection}/tst.js (100%) rename javascript/ql/test/query-tests/Security/CWE-400/{RemovePropertyInjection => RemotePropertyInjection}/tstNonExpr.js (100%) diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected rename to javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.qlref b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.qlref similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.qlref rename to javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.qlref diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tst.js b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tst.js rename to javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tstNonExpr.js b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tstNonExpr.js similarity index 100% rename from javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tstNonExpr.js rename to javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tstNonExpr.js From c39c04cb86c074ae2684c5997bf13d4728cd1d3c Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 27 Aug 2025 10:20:38 +0000 Subject: [PATCH 2/4] JS: added new test case for remote prop injection via `Object.keys` --- .../RemotePropertyInjection.expected | 22 +++++++++---------- .../CWE-400/RemotePropertyInjection/tst.js | 9 ++++++-- 2 files changed, 18 insertions(+), 13 deletions(-) diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected index 9b486b593330..0352205e30b7 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected @@ -11,11 +11,11 @@ edges | tst.js:8:6:8:52 | prop | tst.js:16:10:16:13 | prop | provenance | | | 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:21:25:21:25 | x | provenance | | -| tst.js:21:25:21:25 | x | tst.js:22:15:22:15 | x | provenance | | -| tst.js:22:6:22:15 | result | tst.js:23:9:23:14 | result | provenance | | -| tst.js:22:15:22:15 | x | tst.js:22:6:22:15 | result | provenance | | -| tst.js:23:9:23:14 | result | tst.js:23:9:23:42 | result. ... length) | provenance | | +| tst.js:8:28:8:51 | req.que ... trolled | tst.js:27:25:27:25 | x | 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 | | +| tst.js:29:9:29:14 | result | tst.js:29:9:29:42 | result. ... length) | provenance | | | tstNonExpr.js:5:7:5:23 | userVal | tstNonExpr.js:8:17:8:23 | userVal | provenance | | | tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:5:7:5:23 | userVal | provenance | | nodes @@ -26,13 +26,13 @@ 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:21:25:21:25 | x | semmle.label | x | -| tst.js:22:6:22:15 | result | semmle.label | result | -| tst.js:22:15:22:15 | x | semmle.label | x | -| tst.js:23:9:23:14 | result | semmle.label | result | -| tst.js:23:9:23:42 | result. ... length) | semmle.label | result. ... length) | +| 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 | +| tst.js:29:9:29:14 | result | semmle.label | result | +| tst.js:29:9:29:42 | result. ... length) | semmle.label | result. ... length) | | tstNonExpr.js:5:7:5:23 | userVal | semmle.label | userVal | | tstNonExpr.js:5:17:5:23 | req.url | semmle.label | req.url | | tstNonExpr.js:8:17:8:23 | userVal | semmle.label | userVal | subpaths -| tst.js:8:28:8:51 | req.que ... trolled | tst.js:21:25:21:25 | x | tst.js:23:9:23:42 | result. ... length) | tst.js:8:13:8:52 | myCoolL ... rolled) | +| tst.js:8:28:8:51 | req.que ... trolled | tst.js:27:25:27:25 | x | tst.js:29:9:29:42 | result. ... length) | tst.js:8:13:8:52 | myCoolL ... rolled) | diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js index 301b7b808810..122e9a4c51f0 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js +++ b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js @@ -16,10 +16,15 @@ app.get('/user/:id', function(req, res) { headers[prop] = 42; // $ Alert res.set(headers); myCoolLocalFct[req.query.x](); // OK - flagged by method name injection + + Object.keys(req.body).forEach( // $ MISSING: Source + key => { + myObj[key] = 42; // $ MISSING: Alert + } + ); }); function myCoolLocalFct(x) { var result = x; return result.substring(0, result.length); - -} \ No newline at end of file +} From 32606584ea6bd70063371eef0a47c7a645c3e6ae Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 27 Aug 2025 10:23:03 +0000 Subject: [PATCH 3/4] JS: add enumeration taint flow to Remote Property Injection query --- .../security/dataflow/RemotePropertyInjectionQuery.qll | 5 +++++ .../RemotePropertyInjection.expected | 6 ++++++ .../Security/CWE-400/RemotePropertyInjection/tst.js | 4 ++-- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll index 8f1f174d8ecf..f338651b63de 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll +++ b/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() } } diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected index 0352205e30b7..7021d9b04d11 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected +++ b/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 | diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js b/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js index 122e9a4c51f0..ebdb07a758b1 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js +++ b/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 } ); }); From e0916c8750b4f0d72092a390eb85335c2d9e621b Mon Sep 17 00:00:00 2001 From: Napalys Klicius Date: Wed, 27 Aug 2025 10:32:45 +0000 Subject: [PATCH 4/4] JS: add change note --- .../2025-08-27-remote-property-injection-update.md | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md diff --git a/javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md b/javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md new file mode 100644 index 000000000000..17fe6123cceb --- /dev/null +++ b/javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The query `js/remote-property-injection` now detects property injection vulnerabilities through object enumeration patterns such as `Object.keys()`. \ No newline at end of file