hohn/codeql
1
0
Fork 0
mirror of https://github.com/github/codeql.git synced 2026-05-01 03:35:13 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge branch 'main' into expand-ruby-ssrf-sinks-faraday-connection-new

Browse Source
This commit is contained in:
thiggy1342
2022-10-25 13:09:17 -04:00
committed by GitHub
parent 3659eaa780 3d025ea77e
commit 9c1fbfd330
70 changed files with 1659 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
ruby/ql/lib/codeql/ruby/Concepts.qll
View File

@@ -701,6 +701,9 @@ module SystemCommandExecution {
class CodeExecution extends DataFlow::Node instanceof CodeExecution::Range {
/** Gets the argument that specifies the code to be executed. */
DataFlow::Node getCode() { result = super.getCode() }
/** Holds if this execution runs arbitrary code, as opposed to some restricted subset. E.g. `Object.send` will only run any method on an object. */
predicate runsArbitraryCode() { super.runsArbitraryCode() }
}
/** Provides a class for modeling new dynamic code execution APIs. */
@@ -714,6 +717,9 @@ module CodeExecution {
abstract class Range extends DataFlow::Node {
/** Gets the argument that specifies the code to be executed. */
abstract DataFlow::Node getCode();
/** Holds if this execution runs arbitrary code, as opposed to some restricted subset. E.g. `Object.send` will only run any method on an object. */
predicate runsArbitraryCode() { any() }
}
}

2
ruby/ql/lib/codeql/ruby/frameworks/ActiveJob.qll
View File

@@ -25,6 +25,8 @@ module ActiveJob {
}
override DataFlow::Node getCode() { result = this.getArgument(0) }
override predicate runsArbitraryCode() { none() }
}
}
}

2
ruby/ql/lib/codeql/ruby/frameworks/ActiveStorage.qll
View File

@@ -221,5 +221,7 @@ module ActiveStorage {
}
override DataFlow::Node getCode() { result = this.getArgument(0) }
override predicate runsArbitraryCode() { none() }
}
}

2
ruby/ql/lib/codeql/ruby/frameworks/ActiveSupport.qll
View File

@@ -35,6 +35,8 @@ module ActiveSupport {
}
override DataFlow::Node getCode() { result = this.getReceiver() }
override predicate runsArbitraryCode() { none() }
}
/**

2
ruby/ql/lib/codeql/ruby/frameworks/core/Kernel.qll
View File

@@ -166,6 +166,8 @@ module Kernel {
SendCallCodeExecution() { this.getMethodName() = "send" }
override DataFlow::Node getCode() { result = this.getArgument(0) }
override predicate runsArbitraryCode() { none() }
}
private class TapSummary extends SimpleSummarizedCallable {

2
ruby/ql/lib/codeql/ruby/frameworks/core/Module.qll
View File

@@ -42,5 +42,7 @@ module Module {
}
override DataFlow::Node getCode() { result = this.getArgument(0) }
override predicate runsArbitraryCode() { none() }
}
}

58
ruby/ql/lib/codeql/ruby/security/CodeInjectionCustomizations.qll
View File

@@ -11,20 +11,41 @@ private import codeql.ruby.dataflow.BarrierGuards
* adding your own.
*/
module CodeInjection {
/** Flow states used to distinguish whether an attacker controls the entire string. */
module FlowState {
/** Flow state used for normal tainted data, where an attacker might only control a substring. */
DataFlow::FlowState substring() { result = "substring" }
/** Flow state used for data that is entirely controlled by the attacker. */
DataFlow::FlowState full() { result = "full" }
}
/**
* A data flow source for "Code injection" vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }
abstract class Source extends DataFlow::Node {
/** Gets a flow state for which this is a source. */
DataFlow::FlowState getAFlowState() { result = [FlowState::substring(), FlowState::full()] }
}
/**
* A data flow sink for "Code injection" vulnerabilities.
*/
abstract class Sink extends DataFlow::Node { }
abstract class Sink extends DataFlow::Node {
/** Holds if this sink is safe for an attacker that only controls a substring. */
DataFlow::FlowState getAFlowState() { result = [FlowState::substring(), FlowState::full()] }
}
/**
* A sanitizer for "Code injection" vulnerabilities.
*/
abstract class Sanitizer extends DataFlow::Node { }
abstract class Sanitizer extends DataFlow::Node {
/**
* Gets a flow state for which this is a sanitizer.
* Sanitizes all states if the result is empty.
*/
DataFlow::FlowState getAFlowState() { none() }
}
/**
* DEPRECATED: Use `Sanitizer` instead.
@@ -42,6 +63,35 @@ module CodeInjection {
* A call that evaluates its arguments as Ruby code, considered as a flow sink.
*/
class CodeExecutionAsSink extends Sink {
CodeExecutionAsSink() { this = any(CodeExecution c).getCode() }
CodeExecution c;
CodeExecutionAsSink() { this = c.getCode() }
/** Gets a flow state for which this is a sink. */
override DataFlow::FlowState getAFlowState() {
if c.runsArbitraryCode()
then result = [FlowState::substring(), FlowState::full()] // If it runs immediately, then it's always vulnerable.
else result = FlowState::full() // If it "just" loads something, then it's only vulnerable if the attacker controls the entire string.
}
}
private import codeql.ruby.AST as Ast
/**
* A string-concatenation that sanitizes the `full()` state.
*/
class StringConcatenationSanitizer extends Sanitizer {
StringConcatenationSanitizer() {
// string concatenations sanitize the `full` state, as an attacker no longer controls the entire string
exists(Ast::AstNode str |
str instanceof Ast::StringLiteral
or
str instanceof Ast::AddExpr
|
this.asExpr().getExpr() = str
)
}
override DataFlow::FlowState getAFlowState() { result = FlowState::full() }
}
}

18
ruby/ql/lib/codeql/ruby/security/CodeInjectionQuery.qll
View File

@@ -16,16 +16,26 @@ import codeql.ruby.dataflow.BarrierGuards
class Configuration extends TaintTracking::Configuration {
Configuration() { this = "CodeInjection" }
override predicate isSource(DataFlow::Node source) { source instanceof Source }
override predicate isSource(DataFlow::Node source, DataFlow::FlowState state) {
state = source.(Source).getAFlowState()
}
override predicate isSink(DataFlow::Node sink) { sink instanceof Sink }
override predicate isSink(DataFlow::Node sink, DataFlow::FlowState state) {
state = sink.(Sink).getAFlowState()
}
override predicate isSanitizer(DataFlow::Node node) {
node instanceof Sanitizer or
node instanceof StringConstCompareBarrier or
node instanceof Sanitizer and not exists(node.(Sanitizer).getAFlowState())
or
node instanceof StringConstCompareBarrier
or
node instanceof StringConstArrayInclusionCallBarrier
}
override predicate isSanitizer(DataFlow::Node node, DataFlow::FlowState state) {
node.(Sanitizer).getAFlowState() = state
}
deprecated override predicate isSanitizerGuard(DataFlow::BarrierGuard guard) {
guard instanceof SanitizerGuard
}

9
ruby/ql/src/queries/security/cwe-094/CodeInjection.ql
View File

@@ -21,6 +21,13 @@ import DataFlow::PathGraph
from Configuration config, DataFlow::PathNode source, DataFlow::PathNode sink, Source sourceNode
where
config.hasFlowPath(source, sink) and
sourceNode = source.getNode()
sourceNode = source.getNode() and
// removing duplications of the same path, but different flow-labels.
sink =
min(DataFlow::PathNode otherSink |
config.hasFlowPath(any(DataFlow::PathNode s | s.getNode() = source.getNode()), otherSink)
|
otherSink order by otherSink.getState()
)
select sink.getNode(), source, sink, "This code execution depends on a $@.", source.getNode(),
"user-provided value"

20
ruby/ql/test/query-tests/security/cwe-094/CodeInjection.expected
View File

@@ -1,25 +1,44 @@
edges
| CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:5:12:5:24 | ...[...] : |
| CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:5:12:5:24 | ...[...] : |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:8:10:8:13 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:8:10:8:13 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:20:20:20:23 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:20:20:20:23 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:23:21:23:24 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:23:21:23:24 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:29:15:29:18 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:32:19:32:22 | code |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:38:24:38:27 | code : |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:38:24:38:27 | code : |
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:41:40:41:43 | code |
| CodeInjection.rb:38:24:38:27 | code : | CodeInjection.rb:38:10:38:28 | call to escape |
| CodeInjection.rb:38:24:38:27 | code : | CodeInjection.rb:38:10:38:28 | call to escape |
| CodeInjection.rb:78:12:78:17 | call to params : | CodeInjection.rb:78:12:78:24 | ...[...] : |
| CodeInjection.rb:78:12:78:24 | ...[...] : | CodeInjection.rb:80:16:80:19 | code |
nodes
| CodeInjection.rb:5:12:5:17 | call to params : | semmle.label | call to params : |
| CodeInjection.rb:5:12:5:17 | call to params : | semmle.label | call to params : |
| CodeInjection.rb:5:12:5:24 | ...[...] : | semmle.label | ...[...] : |
| CodeInjection.rb:5:12:5:24 | ...[...] : | semmle.label | ...[...] : |
| CodeInjection.rb:8:10:8:13 | code | semmle.label | code |
| CodeInjection.rb:8:10:8:13 | code | semmle.label | code |
| CodeInjection.rb:11:10:11:15 | call to params | semmle.label | call to params |
| CodeInjection.rb:11:10:11:15 | call to params | semmle.label | call to params |
| CodeInjection.rb:20:20:20:23 | code | semmle.label | code |
| CodeInjection.rb:20:20:20:23 | code | semmle.label | code |
| CodeInjection.rb:23:21:23:24 | code | semmle.label | code |
| CodeInjection.rb:23:21:23:24 | code | semmle.label | code |
| CodeInjection.rb:29:15:29:18 | code | semmle.label | code |
| CodeInjection.rb:32:19:32:22 | code | semmle.label | code |
| CodeInjection.rb:38:10:38:28 | call to escape | semmle.label | call to escape |
| CodeInjection.rb:38:10:38:28 | call to escape | semmle.label | call to escape |
| CodeInjection.rb:38:24:38:27 | code : | semmle.label | code : |
| CodeInjection.rb:38:24:38:27 | code : | semmle.label | code : |
| CodeInjection.rb:41:40:41:43 | code | semmle.label | code |
| CodeInjection.rb:78:12:78:17 | call to params : | semmle.label | call to params : |
| CodeInjection.rb:78:12:78:24 | ...[...] : | semmle.label | ...[...] : |
| CodeInjection.rb:80:16:80:19 | code | semmle.label | code |
subpaths
#select
| CodeInjection.rb:8:10:8:13 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:8:10:8:13 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
@@ -30,3 +49,4 @@ subpaths
| CodeInjection.rb:32:19:32:22 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:32:19:32:22 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
| CodeInjection.rb:38:10:38:28 | call to escape | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:38:10:38:28 | call to escape | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
| CodeInjection.rb:41:40:41:43 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:41:40:41:43 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
| CodeInjection.rb:80:16:80:19 | code | CodeInjection.rb:78:12:78:17 | call to params : | CodeInjection.rb:80:16:80:19 | code | This code execution depends on a $@. | CodeInjection.rb:78:12:78:17 | call to params | user-provided value |

12
ruby/ql/test/query-tests/security/cwe-094/CodeInjection.rb
View File

@@ -72,3 +72,15 @@ class Bar
true
end
end
class UsersController < ActionController::Base
def create
code = params[:code]
obj().send(code, "foo"); # BAD
obj().send("prefix_" + code + "_suffix", "foo"); # GOOD
obj().send("prefix_#{code}_suffix", "foo"); # GOOD
end
end