From b985ddb868737eb00c5a5752fdcd53b2556e7e6f Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Mon, 7 Jun 2021 09:56:59 +0200 Subject: [PATCH] Use InlineExpectationsTest --- .../code/java/security/SpelInjection.qll | 2 +- .../security/CWE-094/SpelInjection.qlref | 1 - .../CWE-094/SpelInjectionTest.expected | 0 ...lInjection.java => SpelInjectionTest.java} | 17 +++++----- .../security/CWE-094/SpelInjectionTest.ql | 32 +++++++++++++++++++ 5 files changed, 41 insertions(+), 11 deletions(-) delete mode 100644 java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref create mode 100644 java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.expected rename java/ql/test/query-tests/security/CWE-094/{SpelInjection.java => SpelInjectionTest.java} (86%) create mode 100644 java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql diff --git a/java/ql/src/semmle/code/java/security/SpelInjection.qll b/java/ql/src/semmle/code/java/security/SpelInjection.qll index 39a9d9b19ea..f5435802aa2 100644 --- a/java/ql/src/semmle/code/java/security/SpelInjection.qll +++ b/java/ql/src/semmle/code/java/security/SpelInjection.qll @@ -56,7 +56,7 @@ private class DefaultSpelExpressionInjectionAdditionalTaintStep extends SpelExpr /** * A configuration for safe evaluation context that may be used in expression evaluation. */ -class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration { +private class SafeEvaluationContextFlowConfig extends DataFlow2::Configuration { SafeEvaluationContextFlowConfig() { this = "SpelInjection::SafeEvaluationContextFlowConfig" } override predicate isSource(DataFlow::Node source) { source instanceof SafeContextSource } diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref b/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref deleted file mode 100644 index f949263d4d3..00000000000 --- a/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -Security/CWE/CWE-094/SpelInjection.ql \ No newline at end of file diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.expected b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.expected new file mode 100644 index 00000000000..e69de29bb2d diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjection.java b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.java similarity index 86% rename from java/ql/test/query-tests/security/CWE-094/SpelInjection.java rename to java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.java index 9eeb552ef34..a6bdc956ed4 100644 --- a/java/ql/test/query-tests/security/CWE-094/SpelInjection.java +++ b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.java @@ -7,7 +7,7 @@ import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.expression.spel.support.SimpleEvaluationContext; import org.springframework.expression.spel.support.StandardEvaluationContext; -public class SpelInjection { +public class SpelInjectionTest { private static final ExpressionParser PARSER = new SpelExpressionParser(); @@ -20,7 +20,7 @@ public class SpelInjection { ExpressionParser parser = new SpelExpressionParser(); Expression expression = parser.parseExpression(input); - expression.getValue(); + expression.getValue(); // $hasSpelInjection } public void testGetValueWithChainedCalls(Socket socket) throws IOException { @@ -31,7 +31,7 @@ public class SpelInjection { String input = new String(bytes, 0, n); Expression expression = new SpelExpressionParser().parseExpression(input); - expression.getValue(); + expression.getValue(); // $hasSpelInjection } public void testSetValueWithRootObject(Socket socket) throws IOException { @@ -45,7 +45,7 @@ public class SpelInjection { Object root = new Object(); Object value = new Object(); - expression.setValue(root, value); + expression.setValue(root, value); // $hasSpelInjection } public void testGetValueWithStaticParser(Socket socket) throws IOException { @@ -56,7 +56,7 @@ public class SpelInjection { String input = new String(bytes, 0, n); Expression expression = PARSER.parseExpression(input); - expression.getValue(); + expression.getValue(); // $hasSpelInjection } public void testGetValueType(Socket socket) throws IOException { @@ -67,7 +67,7 @@ public class SpelInjection { String input = new String(bytes, 0, n); Expression expression = PARSER.parseExpression(input); - expression.getValueType(); + expression.getValueType(); // $hasSpelInjection } public void testWithStandardEvaluationContext(Socket socket) throws IOException { @@ -80,7 +80,7 @@ public class SpelInjection { Expression expression = PARSER.parseExpression(input); StandardEvaluationContext context = new StandardEvaluationContext(); - expression.getValue(context); + expression.getValue(context); // $hasSpelInjection } public void testWithSimpleEvaluationContext(Socket socket) throws IOException { @@ -93,8 +93,7 @@ public class SpelInjection { Expression expression = PARSER.parseExpression(input); SimpleEvaluationContext context = SimpleEvaluationContext.forReadWriteDataBinding().build(); - // the expression is evaluated in a limited context - expression.getValue(context); + expression.getValue(context); // Safe - the expression is evaluated in a limited context } } diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql new file mode 100644 index 00000000000..bbc56b33cb7 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-094/SpelInjectionTest.ql @@ -0,0 +1,32 @@ +import java +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.security.SpelInjection +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" } + + override string getARelevantTag() { result = "hasSpelInjection" } + + 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) | + sink.getLocation() = location and + element = sink.toString() and + value = "" + ) + } +}