hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-04-26 01:05:15 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #20296 from Napalys/js/remote-property-injection-update

Browse Source 
JS: Detect property injection via object enumeration patterns
This commit is contained in:
Napalys Klicius
2025-09-03 14:38:24 +02:00
committed by GitHub
parent a9baf34629 8fc81f4263
commit 7cbaa114a3
6 changed files with 33 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll
View File

@@ -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() }
}

4
javascript/ql/src/change-notes/2025-08-27-remote-property-injection-update.md Normal file
View File

@@ -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()`.

28
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.expected
View File

@@ -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:9 | prop | tst.js:9:8:9:11 | prop | provenance | |
@@ -11,11 +12,13 @@ edges
| tst.js:8:6:8:9 | prop | tst.js:16:10:16:13 | prop | provenance | |
| tst.js:8:13:8:52 | myCoolL ... rolled) | tst.js:8:6:8:9 | 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:11 | result | tst.js:23:9:23:14 | result | provenance | |
| tst.js:22:15:22:15 | x | tst.js:22:6:22:11 | 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: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:11 | result | tst.js:29:9:29:14 | result | provenance | |
| tst.js:28:15:28:15 | x | tst.js:28:6:28:11 | result | provenance | |
| tst.js:29:9:29:14 | result | tst.js:29:9:29:42 | result. ... length) | provenance | |
| tstNonExpr.js:5:7:5:13 | userVal | tstNonExpr.js:8:17:8:23 | userVal | provenance | |
| tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:5:7:5:13 | userVal | provenance | |
nodes
@@ -26,13 +29,16 @@ 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:11 | 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: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:11 | 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:13 | 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) |

0
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.qlref → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/RemotePropertyInjection.qlref
View File

9
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tst.js → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tst.js
View File

@@ -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( // $ Source
key => {
myObj[key] = 42; // $ Alert
}
);
});
function myCoolLocalFct(x) {
var result = x;
return result.substring(0, result.length);
}
}

0
javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/tstNonExpr.js → javascript/ql/test/query-tests/Security/CWE-400/RemotePropertyInjection/tstNonExpr.js
View File