From b30c92e69ec7ae1567ff83cd1f148b766dadbe4a Mon Sep 17 00:00:00 2001 From: Tony Torralba Date: Wed, 2 Jun 2021 11:33:01 +0200 Subject: [PATCH] Refactored into MvelInjection.qll using CSV models --- .../src/Security/CWE/CWE-094/MvelInjection.ql | 26 +- .../Security/CWE/CWE-094/MvelInjectionLib.qll | 367 ------------------ .../code/java/security/MvelInjection.qll | 233 +++++++++++ 3 files changed, 257 insertions(+), 369 deletions(-) delete mode 100644 java/ql/src/Security/CWE/CWE-094/MvelInjectionLib.qll create mode 100644 java/ql/src/semmle/code/java/security/MvelInjection.qll diff --git a/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql b/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql index d32c33c343c..b88b017fc7e 100644 --- a/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql +++ b/java/ql/src/Security/CWE/CWE-094/MvelInjection.ql @@ -11,9 +11,31 @@ */ import java -import MvelInjectionLib +import semmle.code.java.dataflow.FlowSources +import semmle.code.java.dataflow.TaintTracking +import semmle.code.java.security.MvelInjection import DataFlow::PathGraph -from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionConfig conf +/** + * A taint-tracking configuration for unsafe user input + * that is used to construct and evaluate a MVEL expression. + */ +class MvelInjectionFlowConfig extends TaintTracking::Configuration { + MvelInjectionFlowConfig() { this = "MvelInjectionFlowConfig" } + + override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } + + override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink } + + override predicate isSanitizer(DataFlow::Node sanitizer) { + sanitizer instanceof MvelInjectionSanitizer + } + + override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { + any(MvelInjectionAdditionalTaintStep c).step(node1, node2) + } +} + +from DataFlow::PathNode source, DataFlow::PathNode sink, MvelInjectionFlowConfig conf where conf.hasFlowPath(source, sink) select sink.getNode(), source, sink, "MVEL injection from $@.", source.getNode(), "this user input" diff --git a/java/ql/src/Security/CWE/CWE-094/MvelInjectionLib.qll b/java/ql/src/Security/CWE/CWE-094/MvelInjectionLib.qll deleted file mode 100644 index a6cf891330f..00000000000 --- a/java/ql/src/Security/CWE/CWE-094/MvelInjectionLib.qll +++ /dev/null @@ -1,367 +0,0 @@ -import java -import semmle.code.java.dataflow.FlowSources -import semmle.code.java.dataflow.TaintTracking - -/** - * A taint-tracking configuration for unsafe user input - * that is used to construct and evaluate a MVEL expression. - */ -class MvelInjectionConfig extends TaintTracking::Configuration { - MvelInjectionConfig() { this = "MvelInjectionConfig" } - - override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource } - - override predicate isSink(DataFlow::Node sink) { sink instanceof MvelEvaluationSink } - - override predicate isAdditionalTaintStep(DataFlow::Node node1, DataFlow::Node node2) { - expressionCompilationStep(node1, node2) or - createExpressionCompilerStep(node1, node2) or - expressionCompilerCompileStep(node1, node2) or - createCompiledAccExpressionStep(node1, node2) or - scriptCompileStep(node1, node2) or - createMvelCompiledScriptStep(node1, node2) or - templateCompileStep(node1, node2) or - createTemplateCompilerStep(node1, node2) - } -} - -/** - * A sink for EL injection vulnerabilities via MVEL, - * i.e. methods that run evaluation of a MVEL expression. - */ -class MvelEvaluationSink extends DataFlow::ExprNode { - MvelEvaluationSink() { - exists(StaticMethodAccess ma, Method m | m = ma.getMethod() | - ( - m instanceof MvelEvalMethod or - m instanceof TemplateRuntimeEvaluationMethod - ) and - ma.getArgument(0) = asExpr() - ) - or - exists(MethodAccess ma, Method m | m = ma.getMethod() | - m instanceof MvelScriptEngineEvaluationMethod and - ma.getArgument(0) = asExpr() - ) - or - exists(MethodAccess ma, Method m | m = ma.getMethod() | - ( - m instanceof ExecutableStatementEvaluationMethod or - m instanceof CompiledExpressionEvaluationMethod or - m instanceof CompiledAccExpressionEvaluationMethod or - m instanceof AccessorEvaluationMethod or - m instanceof CompiledScriptEvaluationMethod or - m instanceof MvelCompiledScriptEvaluationMethod - ) and - ma.getQualifier() = asExpr() - ) - or - exists(StaticMethodAccess ma, Method m | m = ma.getMethod() | - m instanceof MvelRuntimeEvaluationMethod and - ma.getArgument(1) = asExpr() - ) - } -} - -/** - * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression - * by callilng `MVEL.compileExpression(tainted)`. - */ -predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(StaticMethodAccess ma, Method m | ma.getMethod() = m | - m.getDeclaringType() instanceof MVEL and - m.hasName("compileExpression") and - ma.getAnArgument() = node1.asExpr() and - node2.asExpr() = ma - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step creates `ExpressionCompiler`, - * i.e. `new ExpressionCompiler(tainted)`. - */ -predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(ConstructorCall cc | - cc.getConstructedType() instanceof ExpressionCompiler and - cc = node2.asExpr() and - cc.getArgument(0) = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step creates `CompiledAccExpression`, - * i.e. `new CompiledAccExpression(tainted, ...)`. - */ -predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(ConstructorCall cc | - cc.getConstructedType() instanceof CompiledAccExpression and - cc = node2.asExpr() and - cc.getArgument(0) = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression - * by calling `ExpressionCompiler.compile()`. - */ -predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | ma.getMethod() = m | - m.getDeclaringType() instanceof ExpressionCompiler and - m.hasName("compile") and - ma = node2.asExpr() and - ma.getQualifier() = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that compiles a script via `MvelScriptEngine`, - * i.e. `engine.compile(tainted)` or `engine.compiledScript(tainted)`. - */ -predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | ma.getMethod() = m | - m instanceof MvelScriptEngineCompilationMethod and - ma = node2.asExpr() and - ma.getArgument(0) = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step creates `MvelCompiledScript`, - * i.e. `new MvelCompiledScript(engine, tainted)`. - */ -predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(ConstructorCall cc | - cc.getConstructedType() instanceof MvelCompiledScript and - cc = node2.asExpr() and - cc.getArgument(1) = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step creates `TemplateCompiler`, - * i.e. `new TemplateCompiler(tainted)`. - */ -predicate createTemplateCompilerStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(ConstructorCall cc | - cc.getConstructedType() instanceof TemplateCompiler and - cc = node2.asExpr() and - cc.getArgument(0) = node1.asExpr() - ) -} - -/** - * Holds if `node1` to `node2` is a dataflow step that compiles a script via `TemplateCompiler`, - * i.e. `compiler.compile()` or `TemplateCompiler.compileTemplate(tainted)`. - */ -predicate templateCompileStep(DataFlow::Node node1, DataFlow::Node node2) { - exists(MethodAccess ma, Method m | ma.getMethod() = m | - m instanceof TemplateCompilerCompileMethod and - ma.getQualifier() = node1.asExpr() and - ma = node2.asExpr() - ) - or - exists(StaticMethodAccess ma, Method m | ma.getMethod() = m | - m instanceof TemplateCompilerCompileTemplateMethod and - ma = node2.asExpr() and - ma.getArgument(0) = node1.asExpr() - ) -} - -/** - * Methods in the MVEL class that evaluate a MVEL expression. - */ -class MvelEvalMethod extends Method { - MvelEvalMethod() { - getDeclaringType() instanceof MVEL and - ( - hasName("eval") or - hasName("executeExpression") or - hasName("evalToBoolean") or - hasName("evalToString") or - hasName("executeAllExpression") or - hasName("executeSetExpression") - ) - } -} - -/** - * Methods in `MVEL` class that compile a MVEL expression. - */ -class MvelCompileExpressionMethod extends Method { - MvelCompileExpressionMethod() { - getDeclaringType() instanceof MVEL and - ( - hasName("compileExpression") or - hasName("compileGetExpression") or - hasName("compileSetExpression") - ) - } -} - -/** - * Methods in `ExecutableStatement` that evaluate a MVEL expression. - */ -class ExecutableStatementEvaluationMethod extends Method { - ExecutableStatementEvaluationMethod() { - getDeclaringType() instanceof ExecutableStatement and - hasName("getValue") - } -} - -/** - * Methods in `CompiledExpression` that evaluate a MVEL expression. - */ -class CompiledExpressionEvaluationMethod extends Method { - CompiledExpressionEvaluationMethod() { - getDeclaringType() instanceof CompiledExpression and - hasName("getDirectValue") - } -} - -/** - * Methods in `CompiledAccExpression` that evaluate a MVEL expression. - */ -class CompiledAccExpressionEvaluationMethod extends Method { - CompiledAccExpressionEvaluationMethod() { - getDeclaringType() instanceof CompiledAccExpression and - hasName("getValue") - } -} - -/** - * Methods in `Accessor` that evaluate a MVEL expression. - */ -class AccessorEvaluationMethod extends Method { - AccessorEvaluationMethod() { - getDeclaringType() instanceof Accessor and - hasName("getValue") - } -} - -/** - * Methods in `MvelScriptEngine` that evaluate a MVEL expression. - */ -class MvelScriptEngineEvaluationMethod extends Method { - MvelScriptEngineEvaluationMethod() { - getDeclaringType() instanceof MvelScriptEngine and - (hasName("eval") or hasName("evaluate")) - } -} - -/** - * Methods in `MvelScriptEngine` that compile a MVEL expression. - */ -class MvelScriptEngineCompilationMethod extends Method { - MvelScriptEngineCompilationMethod() { - getDeclaringType() instanceof MvelScriptEngine and - (hasName("compile") or hasName("compiledScript")) - } -} - -/** - * Methods in `CompiledScript` that evaluate a MVEL expression. - */ -class CompiledScriptEvaluationMethod extends Method { - CompiledScriptEvaluationMethod() { - getDeclaringType() instanceof CompiledScript and - hasName("eval") - } -} - -/** - * Methods in `TemplateRuntime` that evaluate a MVEL template. - */ -class TemplateRuntimeEvaluationMethod extends Method { - TemplateRuntimeEvaluationMethod() { - getDeclaringType() instanceof TemplateRuntime and - (hasName("eval") or hasName("execute")) - } -} - -/** - * `TemplateCompiler.compile()` method compiles a MVEL template. - */ -class TemplateCompilerCompileMethod extends Method { - TemplateCompilerCompileMethod() { - getDeclaringType() instanceof TemplateCompiler and - hasName("compile") - } -} - -/** - * `TemplateCompiler.compileTemplate(tainted)` static method compiles a MVEL template. - */ -class TemplateCompilerCompileTemplateMethod extends Method { - TemplateCompilerCompileTemplateMethod() { - getDeclaringType() instanceof TemplateCompiler and - hasName("compileTemplate") - } -} - -/** - * Methods in `MvelCompiledScript` that evaluate a MVEL expression. - */ -class MvelCompiledScriptEvaluationMethod extends Method { - MvelCompiledScriptEvaluationMethod() { - getDeclaringType() instanceof MvelCompiledScript and - hasName("eval") - } -} - -/** - * Methods in `MVELRuntime` that evaluate a MVEL expression. - */ -class MvelRuntimeEvaluationMethod extends Method { - MvelRuntimeEvaluationMethod() { - getDeclaringType() instanceof MVELRuntime and - hasName("execute") - } -} - -class MVEL extends RefType { - MVEL() { hasQualifiedName("org.mvel2", "MVEL") } -} - -class ExpressionCompiler extends RefType { - ExpressionCompiler() { hasQualifiedName("org.mvel2.compiler", "ExpressionCompiler") } -} - -class ExecutableStatement extends RefType { - ExecutableStatement() { hasQualifiedName("org.mvel2.compiler", "ExecutableStatement") } -} - -class CompiledExpression extends RefType { - CompiledExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledExpression") } -} - -class CompiledAccExpression extends RefType { - CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") } -} - -class Accessor extends RefType { - Accessor() { hasQualifiedName("org.mvel2.compiler", "Accessor") } -} - -class CompiledScript extends RefType { - CompiledScript() { hasQualifiedName("javax.script", "CompiledScript") } -} - -class MvelScriptEngine extends RefType { - MvelScriptEngine() { hasQualifiedName("org.mvel2.jsr223", "MvelScriptEngine") } -} - -class MvelCompiledScript extends RefType { - MvelCompiledScript() { hasQualifiedName("org.mvel2.jsr223", "MvelCompiledScript") } -} - -class TemplateRuntime extends RefType { - TemplateRuntime() { hasQualifiedName("org.mvel2.templates", "TemplateRuntime") } -} - -class TemplateCompiler extends RefType { - TemplateCompiler() { hasQualifiedName("org.mvel2.templates", "TemplateCompiler") } -} - -class MVELRuntime extends RefType { - MVELRuntime() { hasQualifiedName("org.mvel2", "MVELRuntime") } -} diff --git a/java/ql/src/semmle/code/java/security/MvelInjection.qll b/java/ql/src/semmle/code/java/security/MvelInjection.qll new file mode 100644 index 00000000000..18bb16c6a65 --- /dev/null +++ b/java/ql/src/semmle/code/java/security/MvelInjection.qll @@ -0,0 +1,233 @@ +/** Provides classes to reason about MVEL injection attacks. */ + +import java +import semmle.code.java.dataflow.DataFlow +import semmle.code.java.dataflow.ExternalFlow + +/** A data flow sink for unvalidated user input that is used to construct MVEL expressions. */ +abstract class MvelEvaluationSink extends DataFlow::Node { } + +/** A sanitizer that prevents MVEL injection attacks. */ +abstract class MvelInjectionSanitizer extends DataFlow::Node { } + +/** + * A unit class for adding additional taint steps. + * + * Extend this class to add additional taint steps that should apply to the `MvelInjectionFlowConfig`. + */ +class MvelInjectionAdditionalTaintStep extends Unit { + /** + * Holds if the step from `node1` to `node2` should be considered a taint + * step for the `MvelInjectionFlowConfig` configuration. + */ + abstract predicate step(DataFlow::Node n1, DataFlow::Node n2); +} + +private class DefaulMvelEvaluationSinkModel extends SinkModelCsv { + override predicate row(string row) { + row = + [ + "javax.script;CompiledScript;false;eval;;;Argument[-1];mvel", + "org.mvel2;MVEL;false;eval;;;Argument[0];mvel", + "org.mvel2;MVEL;false;executeExpression;;;Argument[0];mvel", + "org.mvel2;MVEL;false;evalToBoolean;;;Argument[0];mvel", + "org.mvel2;MVEL;false;evalToString;;;Argument[0];mvel", + "org.mvel2;MVEL;false;executeAllExpression;;;Argument[0];mvel", + "org.mvel2;MVEL;false;executeSetExpression;;;Argument[0];mvel", + "org.mvel2;MVELRuntime;false;execute;;;Argument[1];mvel", + "org.mvel2.templates;TemplateRuntime;false;eval;;;Argument[0];mvel", + "org.mvel2.templates;TemplateRuntime;false;execute;;;Argument[0];mvel", + "org.mvel2.jsr223;MvelScriptEngine;false;eval;;;Argument[0];mvel", + "org.mvel2.jsr223;MvelScriptEngine;false;evaluate;;;Argument[0];mvel", + "org.mvel2.jsr223;MvelCompiledScript;false;eval;;;Argument[-1];mvel", + "org.mvel2.compiler;ExecutableStatement;false;getValue;;;Argument[-1];mvel", + "org.mvel2.compiler;CompiledExpression;false;getDirectValue;;;Argument[-1];mvel", + "org.mvel2.compiler;CompiledAccExpression;false;getValue;;;Argument[-1];mvel", + "org.mvel2.compiler;Accessor;false;getValue;;;Argument[-1];mvel" + ] + } +} + +/** Default sink for MVEL injection vulnerabilities. */ +private class DefaultMvelEvaluationSink extends MvelEvaluationSink { + DefaultMvelEvaluationSink() { sinkNode(this, "mvel") } +} + +/** A default sanitizer that considers numeric and boolean typed data safe for building MVEL expressions */ +private class DefaultMvelInjectionSanitizer extends MvelInjectionSanitizer { + DefaultMvelInjectionSanitizer() { + this.getType() instanceof NumericType or this.getType() instanceof BooleanType + } +} + +/** A set of additional taint steps to consider when taint tracking MVEL related data flows. */ +private class DefaultMvelInjectionAdditionalTaintStep extends MvelInjectionAdditionalTaintStep { + override predicate step(DataFlow::Node node1, DataFlow::Node node2) { + expressionCompilationStep(node1, node2) or + createExpressionCompilerStep(node1, node2) or + expressionCompilerCompileStep(node1, node2) or + createCompiledAccExpressionStep(node1, node2) or + scriptCompileStep(node1, node2) or + createMvelCompiledScriptStep(node1, node2) or + templateCompileStep(node1, node2) or + createTemplateCompilerStep(node1, node2) + } +} + +/** + * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression + * by callilng `MVEL.compileExpression(tainted)`. + */ +private predicate expressionCompilationStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(StaticMethodAccess ma, Method m | ma.getMethod() = m | + m.getDeclaringType() instanceof MVEL and + m.hasName("compileExpression") and + ma.getAnArgument() = node1.asExpr() and + node2.asExpr() = ma + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step creates `ExpressionCompiler`, + * i.e. `new ExpressionCompiler(tainted)`. + */ +private predicate createExpressionCompilerStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(ConstructorCall cc | + cc.getConstructedType() instanceof ExpressionCompiler and + cc = node2.asExpr() and + cc.getArgument(0) = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step creates `CompiledAccExpression`, + * i.e. `new CompiledAccExpression(tainted, ...)`. + */ +private predicate createCompiledAccExpressionStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(ConstructorCall cc | + cc.getConstructedType() instanceof CompiledAccExpression and + cc = node2.asExpr() and + cc.getArgument(0) = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that compiles a MVEL expression + * by calling `ExpressionCompiler.compile()`. + */ +private predicate expressionCompilerCompileStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | ma.getMethod() = m | + m.getDeclaringType() instanceof ExpressionCompiler and + m.hasName("compile") and + ma = node2.asExpr() and + ma.getQualifier() = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that compiles a script via `MvelScriptEngine`, + * i.e. `engine.compile(tainted)` or `engine.compiledScript(tainted)`. + */ +private predicate scriptCompileStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | ma.getMethod() = m | + m instanceof MvelScriptEngineCompilationMethod and + ma = node2.asExpr() and + ma.getArgument(0) = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step creates `MvelCompiledScript`, + * i.e. `new MvelCompiledScript(engine, tainted)`. + */ +private predicate createMvelCompiledScriptStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(ConstructorCall cc | + cc.getConstructedType() instanceof MvelCompiledScript and + cc = node2.asExpr() and + cc.getArgument(1) = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step creates `TemplateCompiler`, + * i.e. `new TemplateCompiler(tainted)`. + */ +private predicate createTemplateCompilerStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(ConstructorCall cc | + cc.getConstructedType() instanceof TemplateCompiler and + cc = node2.asExpr() and + cc.getArgument(0) = node1.asExpr() + ) +} + +/** + * Holds if `node1` to `node2` is a dataflow step that compiles a script via `TemplateCompiler`, + * i.e. `compiler.compile()` or `TemplateCompiler.compileTemplate(tainted)`. + */ +private predicate templateCompileStep(DataFlow::Node node1, DataFlow::Node node2) { + exists(MethodAccess ma, Method m | ma.getMethod() = m | + m instanceof TemplateCompilerCompileMethod and + ma.getQualifier() = node1.asExpr() and + ma = node2.asExpr() + ) + or + exists(StaticMethodAccess ma, Method m | ma.getMethod() = m | + m instanceof TemplateCompilerCompileTemplateMethod and + ma = node2.asExpr() and + ma.getArgument(0) = node1.asExpr() + ) +} + +/** + * Methods in `MvelScriptEngine` that compile a MVEL expression. + */ +private class MvelScriptEngineCompilationMethod extends Method { + MvelScriptEngineCompilationMethod() { + getDeclaringType() instanceof MvelScriptEngine and + (hasName("compile") or hasName("compiledScript")) + } +} + +/** + * `TemplateCompiler.compile()` method that compiles a MVEL template. + */ +private class TemplateCompilerCompileMethod extends Method { + TemplateCompilerCompileMethod() { + getDeclaringType() instanceof TemplateCompiler and + hasName("compile") + } +} + +/** + * `TemplateCompiler.compileTemplate(tainted)` static method that compiles a MVEL template. + */ +private class TemplateCompilerCompileTemplateMethod extends Method { + TemplateCompilerCompileTemplateMethod() { + getDeclaringType() instanceof TemplateCompiler and + hasName("compileTemplate") + } +} + +private class MVEL extends RefType { + MVEL() { hasQualifiedName("org.mvel2", "MVEL") } +} + +private class ExpressionCompiler extends RefType { + ExpressionCompiler() { hasQualifiedName("org.mvel2.compiler", "ExpressionCompiler") } +} + +private class CompiledAccExpression extends RefType { + CompiledAccExpression() { hasQualifiedName("org.mvel2.compiler", "CompiledAccExpression") } +} + +private class MvelScriptEngine extends RefType { + MvelScriptEngine() { hasQualifiedName("org.mvel2.jsr223", "MvelScriptEngine") } +} + +private class MvelCompiledScript extends RefType { + MvelCompiledScript() { hasQualifiedName("org.mvel2.jsr223", "MvelCompiledScript") } +} + +private class TemplateCompiler extends RefType { + TemplateCompiler() { hasQualifiedName("org.mvel2.templates", "TemplateCompiler") } +}