Add query for Code Injection

This query finds cases where user input flows to an argument to `eval`
or `send`, which can execute arbitrary Ruby code.
This commit is contained in:
Harry Maclean
2021-09-14 10:58:25 +01:00
parent 916b844557
commit 95e50cedad
13 changed files with 294 additions and 0 deletions

View File

@@ -326,6 +326,7 @@ class SystemCommandExecution extends DataFlow::Node instanceof SystemCommandExec
DataFlow::Node getAnArgument() { result = super.getAnArgument() }
}
/** Provides a class for modeling new operating system command APIs. */
module SystemCommandExecution {
/**
* A data flow node that executes an operating system command, for instance by spawning a new
@@ -342,3 +343,28 @@ module SystemCommandExecution {
predicate isShellInterpreted(DataFlow::Node arg) { none() }
}
}
/**
* A data-flow node that dynamically executes Ruby code.
*
* Extend this class to refine existing API models. If you want to model new APIs,
* extend `CodeExecution::Range` instead.
*/
class CodeExecution extends DataFlow::Node instanceof CodeExecution::Range {
/** Gets the argument that specifies the code to be executed. */
DataFlow::Node getCode() { result = super.getCode() }
}
/** Provides a class for modeling new dynamic code execution APIs. */
module CodeExecution {
/**
* A data-flow node that dynamically executes Ruby code.
*
* Extend this class to model new APIs. If you want to refine existing API models,
* extend `CodeExecution` instead.
*/
abstract class Range extends DataFlow::Node {
/** Gets the argument that specifies the code to be executed. */
abstract DataFlow::Node getCode();
}
}

View File

@@ -243,3 +243,39 @@ class Open3PipelineCall extends SystemCommandExecution::Range {
arg.asExpr().getExpr() = methodCall.getAnArgument()
}
}
/**
* A call to `Kernel.eval`, which executes its argument as Ruby code.
* ```ruby
* a = 1
* Kernel.eval("a = 2")
* a # => 2
* ```
*/
class EvalCallCodeExecution extends CodeExecution::Range {
KernelMethodCall methodCall;
EvalCallCodeExecution() {
this.asExpr().getExpr() = methodCall and methodCall.getMethodName() = "eval"
}
override DataFlow::Node getCode() { result.asExpr().getExpr() = methodCall.getAnArgument() }
}
/**
* A call to `Kernel#send`, which executes its arguments as a Ruby method call.
* ```ruby
* arr = []
* arr.send("push", 1)
* arr # => [1]
* ```
*/
class SendCallCodeExecution extends CodeExecution::Range {
KernelMethodCall methodCall;
SendCallCodeExecution() {
this.asExpr().getExpr() = methodCall and methodCall.getMethodName() = "send"
}
override DataFlow::Node getCode() { result.asExpr().getExpr() = methodCall.getAnArgument() }
}

View File

@@ -0,0 +1,40 @@
private import ruby
private import codeql.ruby.DataFlow
private import codeql.ruby.Concepts
private import codeql.ruby.Frameworks
private import codeql.ruby.dataflow.RemoteFlowSources
private import codeql.ruby.dataflow.BarrierGuards
/**
* Provides default sources, sinks and sanitizers for detecting
* "Code injection" vulnerabilities, as well as extension points for
* adding your own.
*/
module CodeInjection {
/**
* A data flow source for "Code injection" vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }
/**
* A data flow sink for "Code injection" vulnerabilities.
*/
abstract class Sink extends DataFlow::Node { }
/**
* A sanitizer guard for "Code injection" vulnerabilities.
*/
abstract class SanitizerGuard extends DataFlow::BarrierGuard { }
/**
* A source of remote user input, considered as a flow source.
*/
class RemoteFlowSourceAsSource extends Source, RemoteFlowSource { }
/**
* A call that evaluates its arguments as Ruby code, considered as a flow sink.
*/
class CodeExecutionAsSink extends Sink {
CodeExecutionAsSink() { exists(CodeExecution c | this = c.getCode()) }
}
}

View File

@@ -0,0 +1,30 @@
/**
* Provides a taint-tracking configuration for detecting "Code injection" vulnerabilities.
*
* Note, for performance reasons: only import this file if `Configuration` is needed,
* otherwise `CodeInjectionCustomizations` should be imported instead.
*/
private import ruby
import codeql.ruby.DataFlow::DataFlow::PathGraph
import codeql.ruby.DataFlow
import codeql.ruby.TaintTracking
import CodeInjectionCustomizations::CodeInjection
import codeql.ruby.dataflow.BarrierGuards
/**
* A taint-tracking configuration for detecting "Code injection" vulnerabilities.
*/
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "CodeInjection" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof SanitizerGuard or
guard instanceof StringConstCompare or
guard instanceof StringConstArrayInclusionCall
}
}

View File

@@ -0,0 +1,46 @@
<!DOCTYPE qhelp PUBLIC
"-//Semmle//qhelp//EN"
"qhelp.dtd">
<qhelp>
<overview>
<p>
Directly evaluating user input (for example, an HTTP request parameter) as code without first
sanitizing the input allows an attacker arbitrary code execution. This can occur when user
input is passed to code that interprets it as an expression to be
evaluated, using methods such as <code>Kernel.eval</code> or <code>Kernel.send</code>.
</p>
</overview>
<recommendation>
<p>
Avoid including user input in any expression that may be dynamically evaluated. If user input must
be included, use context-specific escaping before including it.
It is important that the correct escaping is used for the type of evaluation that will occur.
</p>
</recommendation>
<example>
<p>
The following example shows two functions setting a name from a request.
The first function uses <code>eval</code> to execute the <code>set_name</code> method.
This is dangerous as it can allow a malicious user to execute arbitrary code on the server.
For example, the user could supply the value <code>"' + exec('rm -rf') + '"</code>
to destroy the server's file system.
The second function calls the <code>set_name</code> method directly and is thus safe.
</p>
<sample src="examples/code_injection.rb" />
</example>
<references>
<li>
OWASP:
<a href="https://www.owasp.org/index.php/Code_Injection">Code Injection</a>.
</li>
<li>
Wikipedia: <a href="https://en.wikipedia.org/wiki/Code_injection">Code Injection</a>.
</li>
</references>
</qhelp>

View File

@@ -0,0 +1,27 @@
/**
* @name Code injection
* @description Interpreting unsanitized user input as code allows a malicious user to perform arbitrary
* code execution.
* @kind path-problem
* @problem.severity error
* @security-severity 9.3
* @sub-severity high
* @precision high
* @id rb/code-injection
* @tags security
* external/owasp/owasp-a1
* external/cwe/cwe-094
* external/cwe/cwe-095
* external/cwe/cwe-116
*/
import ruby
import codeql.ruby.security.CodeInjectionQuery
import DataFlow::PathGraph
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink, Source sourceNode
where
config.hasFlowPath(source, sink) and
sourceNode = source.getNode()
select sink.getNode(), source, sink, "This code execution depends on $@.", sourceNode,
"a user-provided value"

View File

@@ -0,0 +1,17 @@
class UsersController < ActionController::Base
# BAD - Allow user to define code to be run.
def create_bad
first_name = params[:first_name]
eval("set_name(#{first_name})")
end
# GOOD - Call code directly
def create_good
first_name = params[:first_name]
set_name(first_name)
end
def set_name(name)
@name = name
end
end

View File

@@ -0,0 +1,23 @@
# Uses of eval and send
eval("raise \"error\"")
send("raise", "error")
a = []
a.send("raise", "error")
class Foo
def eval(x)
x + 1
end
def send(*args)
2
end
def run
eval("exit 1")
end
end
Foo.new.send("exit", 1)

View File

@@ -58,3 +58,8 @@ open3PipelineCallExecutions
| CommandExecution.rb:63:1:63:40 | call to pipeline_w |
| CommandExecution.rb:64:1:64:44 | call to pipeline_start |
| CommandExecution.rb:65:1:65:38 | call to pipeline |
evalCallCodeExecutions
| Eval.rb:3:1:3:23 | call to eval |
sendCallCodeExecutions
| Eval.rb:4:1:4:22 | call to send |
| Eval.rb:7:1:7:24 | call to send |

View File

@@ -13,3 +13,7 @@ query predicate kernelSpawnCallExecutions(KernelSpawnCall c) { any() }
query predicate open3CallExecutions(Open3Call c) { any() }
query predicate open3PipelineCallExecutions(Open3PipelineCall c) { any() }
query predicate evalCallCodeExecutions(EvalCallCodeExecution e) { any() }
query predicate sendCallCodeExecutions(SendCallCodeExecution e) { any() }

View File

@@ -0,0 +1,10 @@
edges
| CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:6:10:6:13 | code |
nodes
| CodeInjection.rb:3:12:3:17 | call to params : | semmle.label | call to params : |
| CodeInjection.rb:6:10:6:13 | code | semmle.label | code |
| CodeInjection.rb:9:10:9:15 | call to params | semmle.label | call to params |
subpaths
#select
| CodeInjection.rb:6:10:6:13 | code | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:6:10:6:13 | code | This code execution depends on $@. | CodeInjection.rb:3:12:3:17 | call to params | a user-provided value |
| CodeInjection.rb:9:10:9:15 | call to params | CodeInjection.rb:9:10:9:15 | call to params | CodeInjection.rb:9:10:9:15 | call to params | This code execution depends on $@. | CodeInjection.rb:9:10:9:15 | call to params | a user-provided value |

View File

@@ -0,0 +1 @@
queries/security/cwe-094/CodeInjection.ql

View File

@@ -0,0 +1,29 @@
class UsersController < ActionController::Base
def create
code = params[:code]
# BAD
eval(code)
# BAD
eval(params)
# GOOD
Foo.new.bar(code)
end
def update
# GOOD
eval("foo")
end
end
class Foo
def eval(x)
true
end
def bar(x)
eval(x)
end
end