From fc6af0476f4e5e503fe53b4b1cb6f100ba5fdb19 Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Fri, 4 Jun 2021 17:10:23 +0200 Subject: [PATCH] Moved from experimental --- .../SaferSpelExpressionEvaluation.java | 0 .../Security/CWE/CWE-094/SpelInjection.qhelp | 0 .../Security/CWE/CWE-094/SpelInjection.ql | 0 .../Security/CWE/CWE-094/SpelInjectionLib.qll | 5 +- .../CWE/CWE-094/SpringFrameworkLib.qll | 111 ++++++++++++++++++ .../UnsafeSpelExpressionEvaluation.java | 0 .../CWE/CWE-094/SpringFrameworkLib.qll | 109 ----------------- .../security/CWE-094/SpelInjection.qlref | 1 - .../security/CWE-094/SpelInjection.expected | 0 .../security/CWE-094/SpelInjection.java | 0 .../security/CWE-094/SpelInjection.qlref | 1 + 11 files changed, 113 insertions(+), 114 deletions(-) rename java/ql/src/{experimental => }/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java (100%) rename java/ql/src/{experimental => }/Security/CWE/CWE-094/SpelInjection.qhelp (100%) rename java/ql/src/{experimental => }/Security/CWE/CWE-094/SpelInjection.ql (100%) rename java/ql/src/{experimental => }/Security/CWE/CWE-094/SpelInjectionLib.qll (95%) create mode 100644 java/ql/src/Security/CWE/CWE-094/SpringFrameworkLib.qll rename java/ql/src/{experimental => }/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java (100%) delete mode 100644 java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.qlref rename java/ql/test/{experimental => }/query-tests/security/CWE-094/SpelInjection.expected (100%) rename java/ql/test/{experimental => }/query-tests/security/CWE-094/SpelInjection.java (100%) create mode 100644 java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java rename to java/ql/src/Security/CWE/CWE-094/SaferSpelExpressionEvaluation.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpelInjection.qhelp b/java/ql/src/Security/CWE/CWE-094/SpelInjection.qhelp similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-094/SpelInjection.qhelp rename to java/ql/src/Security/CWE/CWE-094/SpelInjection.qhelp diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpelInjection.ql b/java/ql/src/Security/CWE/CWE-094/SpelInjection.ql similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-094/SpelInjection.ql rename to java/ql/src/Security/CWE/CWE-094/SpelInjection.ql diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpelInjectionLib.qll b/java/ql/src/Security/CWE/CWE-094/SpelInjectionLib.qll similarity index 95% rename from java/ql/src/experimental/Security/CWE/CWE-094/SpelInjectionLib.qll rename to java/ql/src/Security/CWE/CWE-094/SpelInjectionLib.qll index 27e0ee463f6..6e37cb5cd82 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/SpelInjectionLib.qll +++ b/java/ql/src/Security/CWE/CWE-094/SpelInjectionLib.qll @@ -10,10 +10,7 @@ import SpringFrameworkLib class ExpressionInjectionConfig extends TaintTracking::Configuration { ExpressionInjectionConfig() { this = "ExpressionInjectionConfig" } - override predicate isSource(DataFlow::Node source) { - source instanceof RemoteFlowSource or - source instanceof WebRequestSource - } + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } override predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionEvaluationSink } diff --git a/java/ql/src/Security/CWE/CWE-094/SpringFrameworkLib.qll b/java/ql/src/Security/CWE/CWE-094/SpringFrameworkLib.qll new file mode 100644 index 00000000000..d240167c398 --- /dev/null +++ b/java/ql/src/Security/CWE/CWE-094/SpringFrameworkLib.qll @@ -0,0 +1,111 @@ +import java +import semmle.code.java.dataflow.DataFlow + +/** + * Methods that trigger evaluation of an expression. + */ +class ExpressionEvaluationMethod extends Method { + ExpressionEvaluationMethod() { + getDeclaringType() instanceof Expression and + ( + hasName("getValue") or + hasName("getValueTypeDescriptor") or + hasName("getValueType") or + hasName("setValue") + ) + } +} + +/** + * Holds if `node1` to `node2` is a dataflow step that converts `PropertyValues` + * to an array of `PropertyValue`, i.e. `tainted.getPropertyValues()`. + */ +predicate getPropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | m = ma.getMethod() | + node1.asExpr() = ma.getQualifier() and + node2.asExpr() = ma and + m.getDeclaringType() instanceof PropertyValues and + m.hasName("getPropertyValues") + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that constructs `MutablePropertyValues`, + * i.e. `new MutablePropertyValues(tainted)`. + */ +predicate createMutablePropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(ConstructorCall cc | cc.getConstructedType() instanceof MutablePropertyValues | + node1.asExpr() = cc.getAnArgument() and + node2.asExpr() = cc + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that returns a name of `PropertyValue`, + * i.e. `tainted.getName()`. + */ +predicate getPropertyNameStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | m = ma.getMethod() | + node1.asExpr() = ma.getQualifier() and + node2.asExpr() = ma and + m.getDeclaringType() instanceof PropertyValue and + m.hasName("getName") + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that converts `MutablePropertyValues` + * to a list of `PropertyValue`, i.e. `tainted.getPropertyValueList()`. + */ +predicate getPropertyValueListStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | m = ma.getMethod() | + node1.asExpr() = ma.getQualifier() and + node2.asExpr() = ma and + m.getDeclaringType() instanceof MutablePropertyValues and + m.hasName("getPropertyValueList") + ) +} + +/** + * Holds if `node1` to `node2` is one of the dataflow steps that propagate + * tainted data via Spring properties. + */ +predicate springPropertiesStep(DataFlow::Node node1, DataFlow::Node node2) { + createMutablePropertyValuesStep(node1, node2) or + getPropertyNameStep(node1, node2) or + getPropertyValuesStep(node1, node2) or + getPropertyValueListStep(node1, node2) +} + +class PropertyValue extends RefType { + PropertyValue() { hasQualifiedName("org.springframework.beans", "PropertyValue") } +} + +class PropertyValues extends RefType { + PropertyValues() { hasQualifiedName("org.springframework.beans", "PropertyValues") } +} + +class MutablePropertyValues extends RefType { + MutablePropertyValues() { hasQualifiedName("org.springframework.beans", "MutablePropertyValues") } +} + +class SimpleEvaluationContext extends RefType { + SimpleEvaluationContext() { + hasQualifiedName("org.springframework.expression.spel.support", "SimpleEvaluationContext") + } +} + +class SimpleEvaluationContextBuilder extends RefType { + SimpleEvaluationContextBuilder() { + hasQualifiedName("org.springframework.expression.spel.support", + "SimpleEvaluationContext$Builder") + } +} + +class Expression extends RefType { + Expression() { hasQualifiedName("org.springframework.expression", "Expression") } +} + +class ExpressionParser extends RefType { + ExpressionParser() { hasQualifiedName("org.springframework.expression", "ExpressionParser") } +} diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java b/java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java similarity index 100% rename from java/ql/src/experimental/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java rename to java/ql/src/Security/CWE/CWE-094/UnsafeSpelExpressionEvaluation.java diff --git a/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll b/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll index dd6ebc43ee7..31c80ea67d8 100644 --- a/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll +++ b/java/ql/src/experimental/Security/CWE/CWE-094/SpringFrameworkLib.qll @@ -1,21 +1,6 @@ import java import semmle.code.java.dataflow.DataFlow -/** - * Methods that trigger evaluation of an expression. - */ -class ExpressionEvaluationMethod extends Method { - ExpressionEvaluationMethod() { - getDeclaringType() instanceof Expression and - ( - hasName("getValue") or - hasName("getValueTypeDescriptor") or - hasName("getValueType") or - hasName("setValue") - ) - } -} - /** * `WebRequest` interface is a source of tainted data. */ @@ -37,100 +22,6 @@ class WebRequestSource extends DataFlow::Node { } } -/** - * Holds if `node1` to `node2` is a dataflow step that converts `PropertyValues` - * to an array of `PropertyValue`, i.e. `tainted.getPropertyValues()`. - */ -predicate getPropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | m = ma.getMethod() | - node1.asExpr() = ma.getQualifier() and - node2.asExpr() = ma and - m.getDeclaringType() instanceof PropertyValues and - m.hasName("getPropertyValues") - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that constructs `MutablePropertyValues`, - * i.e. `new MutablePropertyValues(tainted)`. - */ -predicate createMutablePropertyValuesStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(ConstructorCall cc | cc.getConstructedType() instanceof MutablePropertyValues | - node1.asExpr() = cc.getAnArgument() and - node2.asExpr() = cc - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that returns a name of `PropertyValue`, - * i.e. `tainted.getName()`. - */ -predicate getPropertyNameStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | m = ma.getMethod() | - node1.asExpr() = ma.getQualifier() and - node2.asExpr() = ma and - m.getDeclaringType() instanceof PropertyValue and - m.hasName("getName") - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that converts `MutablePropertyValues` - * to a list of `PropertyValue`, i.e. `tainted.getPropertyValueList()`. - */ -predicate getPropertyValueListStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | m = ma.getMethod() | - node1.asExpr() = ma.getQualifier() and - node2.asExpr() = ma and - m.getDeclaringType() instanceof MutablePropertyValues and - m.hasName("getPropertyValueList") - ) -} - -/** - * Holds if `node1` to `node2` is one of the dataflow steps that propagate - * tainted data via Spring properties. - */ -predicate springPropertiesStep(DataFlow::Node node1, DataFlow::Node node2) { - createMutablePropertyValuesStep(node1, node2) or - getPropertyNameStep(node1, node2) or - getPropertyValuesStep(node1, node2) or - getPropertyValueListStep(node1, node2) -} - -class PropertyValue extends RefType { - PropertyValue() { hasQualifiedName("org.springframework.beans", "PropertyValue") } -} - -class PropertyValues extends RefType { - PropertyValues() { hasQualifiedName("org.springframework.beans", "PropertyValues") } -} - -class MutablePropertyValues extends RefType { - MutablePropertyValues() { hasQualifiedName("org.springframework.beans", "MutablePropertyValues") } -} - -class SimpleEvaluationContext extends RefType { - SimpleEvaluationContext() { - hasQualifiedName("org.springframework.expression.spel.support", "SimpleEvaluationContext") - } -} - -class SimpleEvaluationContextBuilder extends RefType { - SimpleEvaluationContextBuilder() { - hasQualifiedName("org.springframework.expression.spel.support", - "SimpleEvaluationContext$Builder") - } -} - class WebRequest extends RefType { WebRequest() { hasQualifiedName("org.springframework.web.context.request", "WebRequest") } } - -class Expression extends RefType { - Expression() { hasQualifiedName("org.springframework.expression", "Expression") } -} - -class ExpressionParser extends RefType { - ExpressionParser() { hasQualifiedName("org.springframework.expression", "ExpressionParser") } -} diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.qlref b/java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.qlref deleted file mode 100644 index 95bc89c7ae6..00000000000 --- a/java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE/CWE-094/SpelInjection.ql \ No newline at end of file diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.expected b/java/ql/test/query-tests/security/CWE-094/SpelInjection.expected similarity index 100% rename from java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.expected rename to java/ql/test/query-tests/security/CWE-094/SpelInjection.expected diff --git a/java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.java b/java/ql/test/query-tests/security/CWE-094/SpelInjection.java similarity index 100% rename from java/ql/test/experimental/query-tests/security/CWE-094/SpelInjection.java rename to java/ql/test/query-tests/security/CWE-094/SpelInjection.java diff --git a/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref b/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref new file mode 100644 index 00000000000..f949263d4d3 --- /dev/null +++ b/java/ql/test/query-tests/security/CWE-094/SpelInjection.qlref @@ -0,0 +1 @@ +Security/CWE/CWE-094/SpelInjection.ql \ No newline at end of file