From b9bd0520e27799810cf7fff8ad3cea232f5d2784 Mon Sep 17 00:00:00 2001 From: Asger F Date: Thu, 5 Oct 2023 09:21:55 +0200 Subject: [PATCH] JS: Port RemotePropertyInjection --- .../dataflow/RemotePropertyInjectionQuery.qll | 21 +++++++- .../CWE-400/RemotePropertyInjection.ql | 6 +-- .../RemotePropertyInjection.expected | 48 +++++++++---------- 3 files changed, 46 insertions(+), 29 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll b/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll index 83422e8f0de..d3cbfeb8268 100644 --- a/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll +++ b/javascript/ql/lib/semmle/javascript/security/dataflow/RemotePropertyInjectionQuery.qll @@ -14,7 +14,26 @@ import RemotePropertyInjectionCustomizations::RemotePropertyInjection /** * A taint-tracking configuration for reasoning about remote property injection. */ -class Configuration extends TaintTracking::Configuration { +module RemotePropertyInjectionConfig implements DataFlow::ConfigSig { + predicate isSource(DataFlow::Node source) { source instanceof Source } + + predicate isSink(DataFlow::Node sink) { sink instanceof Sink } + + predicate isBarrier(DataFlow::Node node) { + node instanceof Sanitizer or + node = StringConcatenation::getRoot(any(ConstantString str).flow()) + } +} + +/** + * Taint-tracking for reasoning about remote property injection. + */ +module RemotePropertyInjectionFlow = TaintTracking::Global; + +/** + * DEPRECATED. Use the `RemotePropertyInjectionFlow` module instead. + */ +deprecated class Configuration extends TaintTracking::Configuration { Configuration() { this = "RemotePropertyInjection" } override predicate isSource(DataFlow::Node source) { source instanceof Source } diff --git a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.ql b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.ql index 287b196feff..92d18b3f1a2 100644 --- a/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.ql +++ b/javascript/ql/src/Security/CWE-400/RemotePropertyInjection.ql @@ -14,9 +14,9 @@ import javascript import semmle.javascript.security.dataflow.RemotePropertyInjectionQuery -import DataFlow::PathGraph +import RemotePropertyInjectionFlow::PathGraph -from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink -where cfg.hasFlowPath(source, sink) +from RemotePropertyInjectionFlow::PathNode source, RemotePropertyInjectionFlow::PathNode sink +where RemotePropertyInjectionFlow::flowPath(source, sink) select sink.getNode(), source, sink, sink.getNode().(Sink).getMessage() + " depends on a $@.", source.getNode(), "user-provided value" diff --git a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected b/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected index 7907cc41726..d6d347c996d 100644 --- a/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected +++ b/javascript/ql/test/query-tests/Security/CWE-400/RemovePropertyInjection/RemotePropertyInjection.expected @@ -1,37 +1,35 @@ -nodes -| tst.js:8:6:8:52 | prop | -| tst.js:8:13:8:52 | myCoolL ... rolled) | -| tst.js:8:28:8:51 | req.que ... trolled | -| tst.js:8:28:8:51 | req.que ... trolled | -| tst.js:9:8:9:11 | prop | -| tst.js:9:8:9:11 | prop | -| tst.js:13:15:13:18 | prop | -| tst.js:13:15:13:18 | prop | -| tst.js:14:31:14:34 | prop | -| tst.js:14:31:14:34 | prop | -| tst.js:16:10:16:13 | prop | -| tst.js:16:10:16:13 | prop | -| tstNonExpr.js:5:7:5:23 | userVal | -| tstNonExpr.js:5:17:5:23 | req.url | -| tstNonExpr.js:5:17:5:23 | req.url | -| tstNonExpr.js:8:17:8:23 | userVal | -| tstNonExpr.js:8:17:8:23 | userVal | edges | tst.js:8:6:8:52 | prop | tst.js:9:8:9:11 | prop | -| tst.js:8:6:8:52 | prop | tst.js:9:8:9:11 | prop | -| tst.js:8:6:8:52 | prop | tst.js:13:15:13:18 | prop | | tst.js:8:6:8:52 | prop | tst.js:13:15:13:18 | prop | | tst.js:8:6:8:52 | prop | tst.js:14:31:14:34 | prop | -| tst.js:8:6:8:52 | prop | tst.js:14:31:14:34 | prop | -| tst.js:8:6:8:52 | prop | tst.js:16:10:16:13 | prop | | tst.js:8:6:8:52 | prop | tst.js:16:10:16:13 | prop | | tst.js:8:13:8:52 | myCoolL ... rolled) | tst.js:8:6:8:52 | prop | | tst.js:8:28:8:51 | req.que ... trolled | tst.js:8:13:8:52 | myCoolL ... rolled) | -| tst.js:8:28:8:51 | req.que ... trolled | tst.js:8:13:8:52 | myCoolL ... rolled) | -| tstNonExpr.js:5:7:5:23 | userVal | tstNonExpr.js:8:17:8:23 | userVal | +| tst.js:8:28:8:51 | req.que ... trolled | tst.js:21:25:21:25 | x | +| tst.js:21:25:21:25 | x | tst.js:22:15:22:15 | x | +| tst.js:22:6:22:15 | result | tst.js:23:9:23:14 | result | +| tst.js:22:15:22:15 | x | tst.js:22:6:22:15 | result | +| tst.js:23:9:23:14 | result | tst.js:23:9:23:42 | result. ... length) | | tstNonExpr.js:5:7:5:23 | userVal | tstNonExpr.js:8:17:8:23 | userVal | | tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:5:7:5:23 | userVal | -| tstNonExpr.js:5:17:5:23 | req.url | tstNonExpr.js:5:7:5:23 | userVal | +nodes +| tst.js:8:6:8:52 | prop | semmle.label | prop | +| tst.js:8:13:8:52 | myCoolL ... rolled) | semmle.label | myCoolL ... rolled) | +| tst.js:8:28:8:51 | req.que ... trolled | semmle.label | req.que ... trolled | +| tst.js:9:8:9:11 | prop | semmle.label | prop | +| 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) | +| 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) | #select | tst.js:9:8:9:11 | prop | tst.js:8:28:8:51 | req.que ... trolled | tst.js:9:8:9:11 | prop | A property name to write to depends on a $@. | tst.js:8:28:8:51 | req.que ... trolled | user-provided value | | 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 |