diff --git a/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql b/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql index 38038ab05c0..586b265cc73 100644 --- a/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql +++ b/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql @@ -11,25 +11,9 @@ */ import java -import semmle.code.java.security.SpelInjection +import semmle.code.java.security.SpelInjectionQuery import DataFlow::PathGraph -/** - * A taint-tracking configuration for unsafe user input - * that is used to construct and evaluate a SpEL expression. - */ -class SpELInjectionConfig extends TaintTracking::Configuration { - SpELInjectionConfig() { this = "SpELInjectionConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink } - - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { - any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2) - } -} - -from DataFlow::PathNode source, DataFlow::PathNode sink, SpELInjectionConfig conf +from DataFlow::PathNode source, DataFlow::PathNode sink, SpelInjectionConfig conf where conf.hasFlowPath(source, sink) select sink.getNode(), source, sink, "SpEL injection from $@.", source.getNode(), "this user input" diff --git a/java/ql/src/semmle/code/java/security/SpelInjection.qll b/java/ql/src/semmle/code/java/security/SpelInjectionQuery.qll similarity index 88% rename from java/ql/src/semmle/code/java/security/SpelInjection.qll rename to java/ql/src/semmle/code/java/security/SpelInjectionQuery.qll index 420ef66f0dd..c9baa42a587 100644 --- a/java/ql/src/semmle/code/java/security/SpelInjection.qll +++ b/java/ql/src/semmle/code/java/security/SpelInjectionQuery.qll @@ -2,24 +2,11 @@ import java import semmle.code.java.dataflow.DataFlow -import semmle.code.java.dataflow.ExternalFlow import semmle.code.java.dataflow.FlowSources /** A data flow sink for unvalidated user input that is used to construct SpEL expressions. */ abstract class SpelExpressionEvaluationSink extends DataFlow::ExprNode { } -private class SpelExpressionEvaluationModel extends SinkModelCsv { - override predicate row(string row) { - row = - [ - "org.springframework.expression;Expression;true;getValue;;;Argument[-1];spel", - "org.springframework.expression;Expression;true;getValueTypeDescriptor;;;Argument[-1];spel", - "org.springframework.expression;Expression;true;getValueType;;;Argument[-1];spel", - "org.springframework.expression;Expression;true;setValue;;;Argument[-1];spel" - ] - } -} - /** Default sink for SpEL injection vulnerabilities. */ private class DefaultSpelExpressionEvaluationSink extends SpelExpressionEvaluationSink { DefaultSpelExpressionEvaluationSink() { @@ -53,6 +40,22 @@ private class DefaultSpelExpressionInjectionAdditionalTaintStep extends SpelExpr } } +/** + * A taint-tracking configuration for unsafe user input + * that is used to construct and evaluate a SpEL expression. + */ +class SpelInjectionConfig extends TaintTracking::Configuration { + SpelInjectionConfig() { this = "SpelInjectionConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink } + + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2) + } +} + /** * A configuration for safe evaluation context that may be used in expression evaluation. */ diff --git a/java/ql/src/semmle/code/java/security/SpelInjectionSinkModels.qll b/java/ql/src/semmle/code/java/security/SpelInjectionSinkModels.qll new file mode 100644 index 00000000000..c7bf5d1636d --- /dev/null +++ b/java/ql/src/semmle/code/java/security/SpelInjectionSinkModels.qll @@ -0,0 +1,15 @@ +/** Provides sink models relating to SpEL injection vulnerabilities. */ + +import semmle.code.java.dataflow.ExternalFlow + +private class SpelExpressionEvaluationModel extends SinkModelCsv { + override predicate row(string row) { + row = + [ + "org.springframework.expression;Expression;true;getValue;;;Argument[-1];spel", + "org.springframework.expression;Expression;true;getValueTypeDescriptor;;;Argument[-1];spel", + "org.springframework.expression;Expression;true;getValueType;;;Argument[-1];spel", + "org.springframework.expression;Expression;true;setValue;;;Argument[-1];spel" + ] + } +} diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql index bbc56b33cb7..1da99aa3de8 100644 --- a/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql +++ b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql @@ -1,21 +1,9 @@ import java import semmle.code.java.dataflow.TaintTracking import semmle.code.java.dataflow.FlowSources -import semmle.code.java.security.SpelInjection +import semmle.code.java.security.SpelInjectionQuery import TestUtilities.InlineExpectationsTest -class Conf extends TaintTracking::Configuration { - Conf() { this = "test:cwe:spel-injection" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof SpelExpressionEvaluationSink } - - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { - any(SpelExpressionInjectionAdditionalTaintStep c).step(node1, node2) - } -} - class HasSpelInjectionTest extends InlineExpectationsTest { HasSpelInjectionTest() { this = "HasSpelInjectionTest" } @@ -23,7 +11,9 @@ class HasSpelInjectionTest extends InlineExpectationsTest { override predicate hasActualResult(Location location, string element, string tag, string value) { tag = "hasSpelInjection" and - exists(DataFlow::Node src, DataFlow::Node sink, Conf conf | conf.hasFlow(src, sink) | + exists(DataFlow::Node src, DataFlow::Node sink, SpelInjectionConfig conf | + conf.hasFlow(src, sink) + | sink.getLocation() = location and element = sink.toString() and value = ""