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

Merge pull request #11581 from erik-krogh/stdin

Browse Source 
Rb: add stdin as source for unsafe-deserialization
This commit is contained in:
Erik Krogh Kristensen
2023-01-09 13:57:47 +01:00
committed by GitHub
parent e9bbb5d7fa 0a1769657d
commit 5157d4df7b
9 changed files with 98 additions and 72 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
ruby/ql/lib/codeql/ruby/frameworks/Core.qll
View File

@@ -16,6 +16,7 @@ import core.String
import core.Regexp
import core.IO
import core.Digest
import core.Base64
/**
* A system command executed via subshell literal syntax.

25
ruby/ql/lib/codeql/ruby/frameworks/core/Base64.qll Normal file
View File

@@ -0,0 +1,25 @@
/**
* Provides modeling for the `Base64` module.
*/
private import ruby
private import codeql.ruby.dataflow.FlowSummary
private import codeql.ruby.ApiGraphs
private class Base64Decode extends SummarizedCallable {
Base64Decode() { this = "Base64.decode64()" }
override MethodCall getACall() {
result =
API::getTopLevelMember("Base64")
.getAMethodCall(["decode64", "strict_decode64", "urlsafe_decode64"])
.asExpr()
.getExpr()
}
override predicate propagatesFlowExt(string input, string output, boolean preservesValue) {
input = "Argument[0]" and
output = "ReturnValue" and
preservesValue = false
}
}

80
ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll
View File

@@ -10,12 +10,16 @@ private import codeql.ruby.DataFlow
private import codeql.ruby.dataflow.RemoteFlowSources
private import codeql.ruby.frameworks.ActiveJob
private import codeql.ruby.frameworks.core.Module
private import codeql.ruby.frameworks.core.Kernel
module UnsafeDeserialization {
/**
* A data flow source for unsafe deserialization vulnerabilities.
*/
abstract class Source extends DataFlow::Node { }
abstract class Source extends DataFlow::Node {
/** Gets a string that describes the source. */
string describe() { result = "user-provided value" }
}
/**
* A data flow sink for unsafe deserialization vulnerabilities.
@@ -27,16 +31,39 @@ module UnsafeDeserialization {
*/
abstract class Sanitizer extends DataFlow::Node { }
/**
* Additional taint steps for "unsafe deserialization" vulnerabilities.
*/
predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
base64DecodeTaintStep(fromNode, toNode)
}
/** A source of remote user input, considered as a flow source for unsafe deserialization. */
class RemoteFlowSourceAsSource extends Source instanceof RemoteFlowSource { }
/** A read of data from `STDIN`/`ARGV`, considered as a flow source for unsafe deserialization. */
class StdInSource extends UnsafeDeserialization::Source {
boolean stdin;
StdInSource() {
this = API::getTopLevelMember(["STDIN", "ARGF"]).getAMethodCall(["gets", "read"]) and
stdin = true
or
// > $stdin == STDIN
// => true
// but $stdin is special in that it is a global variable and not a constant. `API::getTopLevelMember` only gets constants.
exists(DataFlow::Node dollarStdin |
dollarStdin.asExpr().getExpr().(GlobalVariableReadAccess).getVariable().getName() = "$stdin" and
this = dollarStdin.getALocalSource().getAMethodCall(["gets", "read"])
) and
stdin = true
or
// ARGV.
this.asExpr().getExpr().(GlobalVariableReadAccess).getVariable().getName() = "ARGV" and
stdin = false
or
this.(Kernel::KernelMethodCall).getMethodName() = ["gets", "readline", "readlines"] and
stdin = true
}
override string describe() {
if stdin = true then result = "value from stdin" else result = "value from ARGV"
}
}
/**
* An argument in a call to `Marshal.load` or `Marshal.restore`, considered a
* sink for unsafe deserialization.
@@ -181,41 +208,4 @@ module UnsafeDeserialization {
)
}
}
/**
* `Base64.decode64` propagates taint from its argument to its return value.
*/
predicate base64DecodeTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
exists(DataFlow::CallNode callNode |
callNode =
API::getTopLevelMember("Base64")
.getAMethodCall(["decode64", "strict_decode64", "urlsafe_decode64"])
|
fromNode = callNode.getArgument(0) and
toNode = callNode
)
}
/**
* A argument in a call to `Module.const_get`, considered as a sink for unsafe
* deserialization.
*
* Calls to `Module.const_get` can return arbitrary classes which can then be
* instantiated.
*/
class ConstGetCallArgument extends Sink {
ConstGetCallArgument() { this = any(Module::ModuleConstGetCallCodeExecution c).getCode() }
}
/**
* A argument in a call to `ActiveJob::Serializers.deserialize`, considered as
* a sink for unsafe deserialization.
*
* This is roughly equivalent to a call to `Module.const_get`.
*/
class ActiveJobSerializersDeserializeArgument extends Sink {
ActiveJobSerializersDeserializeArgument() {
this = any(ActiveJob::Serializers::DeserializeCall c).getCode()
}
}
}

4
ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationQuery.qll
View File

@@ -27,8 +27,4 @@ class Configuration extends TaintTracking::Configuration {
super.isSanitizer(node) or
node instanceof UnsafeDeserialization::Sanitizer
}
override predicate isAdditionalTaintStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
UnsafeDeserialization::isAdditionalTaintStep(fromNode, toNode)
}
}

4
ruby/ql/src/change-notes/2023-01-06-stdin-as-deserialization-source.md Normal file
View File

@@ -0,0 +1,4 @@
---
category: minorAnalysis
---
* The `rb/unsafe-deserialization` query now recognizes input from STDIN as a source.

7
ruby/ql/src/queries/security/cwe-502/UnsafeDeserialization.ql
View File

@@ -11,12 +11,11 @@
* external/cwe/cwe-502
*/
import codeql.ruby.AST
import DataFlow::PathGraph
import codeql.ruby.DataFlow
import ruby
import codeql.ruby.security.UnsafeDeserializationQuery
import DataFlow::PathGraph
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
where cfg.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "Unsafe deserialization depends on a $@.", source.getNode(),
"user-provided value"
source.getNode().(UnsafeDeserialization::Source).describe()

1
ruby/ql/test/library-tests/dataflow/local/TaintStep.expected
View File

@@ -2804,6 +2804,7 @@
| file://:0:0:0:0 | parameter position 0 of ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: argument self in ActionController::Parameters#merge! |
| file://:0:0:0:0 | parameter position 0 of ActionController::Parameters#merge! | file://:0:0:0:0 | [summary] to write: return (return) in ActionController::Parameters#merge! |
| file://:0:0:0:0 | parameter position 0 of Arel.sql | file://:0:0:0:0 | [summary] to write: return (return) in Arel.sql |
| file://:0:0:0:0 | parameter position 0 of Base64.decode64() | file://:0:0:0:0 | [summary] to write: return (return) in Base64.decode64() |
| file://:0:0:0:0 | parameter position 0 of File.absolute_path | file://:0:0:0:0 | [summary] to write: return (return) in File.absolute_path |
| file://:0:0:0:0 | parameter position 0 of File.dirname | file://:0:0:0:0 | [summary] to write: return (return) in File.dirname |
| file://:0:0:0:0 | parameter position 0 of File.expand_path | file://:0:0:0:0 | [summary] to write: return (return) in File.expand_path |

26
ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.expected
View File

@@ -1,8 +1,10 @@
edges
| UnsafeDeserialization.rb:10:23:10:50 | call to decode64 : | UnsafeDeserialization.rb:11:27:11:41 | serialized_data |
| UnsafeDeserialization.rb:10:39:10:44 | call to params : | UnsafeDeserialization.rb:10:39:10:50 | ...[...] : |
| UnsafeDeserialization.rb:10:39:10:50 | ...[...] : | UnsafeDeserialization.rb:11:27:11:41 | serialized_data |
| UnsafeDeserialization.rb:10:39:10:50 | ...[...] : | UnsafeDeserialization.rb:10:23:10:50 | call to decode64 : |
| UnsafeDeserialization.rb:16:23:16:50 | call to decode64 : | UnsafeDeserialization.rb:17:30:17:44 | serialized_data |
| UnsafeDeserialization.rb:16:39:16:44 | call to params : | UnsafeDeserialization.rb:16:39:16:50 | ...[...] : |
| UnsafeDeserialization.rb:16:39:16:50 | ...[...] : | UnsafeDeserialization.rb:17:30:17:44 | serialized_data |
| UnsafeDeserialization.rb:16:39:16:50 | ...[...] : | UnsafeDeserialization.rb:16:23:16:50 | call to decode64 : |
| UnsafeDeserialization.rb:22:17:22:22 | call to params : | UnsafeDeserialization.rb:22:17:22:28 | ...[...] : |
| UnsafeDeserialization.rb:22:17:22:28 | ...[...] : | UnsafeDeserialization.rb:23:24:23:32 | json_data |
| UnsafeDeserialization.rb:28:17:28:22 | call to params : | UnsafeDeserialization.rb:28:17:28:28 | ...[...] : |
@@ -18,12 +20,12 @@ edges
| UnsafeDeserialization.rb:81:11:81:22 | ...[...] : | UnsafeDeserialization.rb:82:34:82:36 | xml |
| UnsafeDeserialization.rb:87:17:87:22 | call to params : | UnsafeDeserialization.rb:87:17:87:28 | ...[...] : |
| UnsafeDeserialization.rb:87:17:87:28 | ...[...] : | UnsafeDeserialization.rb:88:25:88:33 | yaml_data |
| UnsafeDeserialization.rb:93:30:93:35 | call to params : | UnsafeDeserialization.rb:93:30:93:43 | ...[...] |
| UnsafeDeserialization.rb:99:48:99:53 | call to params : | UnsafeDeserialization.rb:99:48:99:61 | ...[...] |
nodes
| UnsafeDeserialization.rb:10:23:10:50 | call to decode64 : | semmle.label | call to decode64 : |
| UnsafeDeserialization.rb:10:39:10:44 | call to params : | semmle.label | call to params : |
| UnsafeDeserialization.rb:10:39:10:50 | ...[...] : | semmle.label | ...[...] : |
| UnsafeDeserialization.rb:11:27:11:41 | serialized_data | semmle.label | serialized_data |
| UnsafeDeserialization.rb:16:23:16:50 | call to decode64 : | semmle.label | call to decode64 : |
| UnsafeDeserialization.rb:16:39:16:44 | call to params : | semmle.label | call to params : |
| UnsafeDeserialization.rb:16:39:16:50 | ...[...] : | semmle.label | ...[...] : |
| UnsafeDeserialization.rb:17:30:17:44 | serialized_data | semmle.label | serialized_data |
@@ -49,10 +51,11 @@ nodes
| UnsafeDeserialization.rb:87:17:87:22 | call to params : | semmle.label | call to params : |
| UnsafeDeserialization.rb:87:17:87:28 | ...[...] : | semmle.label | ...[...] : |
| UnsafeDeserialization.rb:88:25:88:33 | yaml_data | semmle.label | yaml_data |
| UnsafeDeserialization.rb:93:30:93:35 | call to params : | semmle.label | call to params : |
| UnsafeDeserialization.rb:93:30:93:43 | ...[...] | semmle.label | ...[...] |
| UnsafeDeserialization.rb:99:48:99:53 | call to params : | semmle.label | call to params : |
| UnsafeDeserialization.rb:99:48:99:61 | ...[...] | semmle.label | ...[...] |
| UnsafeDeserialization.rb:92:24:92:34 | call to read | semmle.label | call to read |
| UnsafeDeserialization.rb:95:24:95:33 | call to gets | semmle.label | call to gets |
| UnsafeDeserialization.rb:98:24:98:32 | call to read | semmle.label | call to read |
| UnsafeDeserialization.rb:101:24:101:27 | call to gets | semmle.label | call to gets |
| UnsafeDeserialization.rb:104:24:104:32 | call to readlines | semmle.label | call to readlines |
subpaths
#select
| UnsafeDeserialization.rb:11:27:11:41 | serialized_data | UnsafeDeserialization.rb:10:39:10:44 | call to params : | UnsafeDeserialization.rb:11:27:11:41 | serialized_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:10:39:10:44 | call to params | user-provided value |
@@ -65,5 +68,8 @@ subpaths
| UnsafeDeserialization.rb:69:23:69:31 | json_data | UnsafeDeserialization.rb:59:17:59:22 | call to params : | UnsafeDeserialization.rb:69:23:69:31 | json_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:59:17:59:22 | call to params | user-provided value |
| UnsafeDeserialization.rb:82:34:82:36 | xml | UnsafeDeserialization.rb:81:11:81:16 | call to params : | UnsafeDeserialization.rb:82:34:82:36 | xml | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:81:11:81:16 | call to params | user-provided value |
| UnsafeDeserialization.rb:88:25:88:33 | yaml_data | UnsafeDeserialization.rb:87:17:87:22 | call to params : | UnsafeDeserialization.rb:88:25:88:33 | yaml_data | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:87:17:87:22 | call to params | user-provided value |
| UnsafeDeserialization.rb:93:30:93:43 | ...[...] | UnsafeDeserialization.rb:93:30:93:35 | call to params : | UnsafeDeserialization.rb:93:30:93:43 | ...[...] | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:93:30:93:35 | call to params | user-provided value |
| UnsafeDeserialization.rb:99:48:99:61 | ...[...] | UnsafeDeserialization.rb:99:48:99:53 | call to params : | UnsafeDeserialization.rb:99:48:99:61 | ...[...] | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:99:48:99:53 | call to params | user-provided value |
| UnsafeDeserialization.rb:92:24:92:34 | call to read | UnsafeDeserialization.rb:92:24:92:34 | call to read | UnsafeDeserialization.rb:92:24:92:34 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:92:24:92:34 | call to read | value from stdin |
| UnsafeDeserialization.rb:95:24:95:33 | call to gets | UnsafeDeserialization.rb:95:24:95:33 | call to gets | UnsafeDeserialization.rb:95:24:95:33 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:95:24:95:33 | call to gets | value from stdin |
| UnsafeDeserialization.rb:98:24:98:32 | call to read | UnsafeDeserialization.rb:98:24:98:32 | call to read | UnsafeDeserialization.rb:98:24:98:32 | call to read | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:98:24:98:32 | call to read | value from stdin |
| UnsafeDeserialization.rb:101:24:101:27 | call to gets | UnsafeDeserialization.rb:101:24:101:27 | call to gets | UnsafeDeserialization.rb:101:24:101:27 | call to gets | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:101:24:101:27 | call to gets | value from stdin |
| UnsafeDeserialization.rb:104:24:104:32 | call to readlines | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | Unsafe deserialization depends on a $@. | UnsafeDeserialization.rb:104:24:104:32 | call to readlines | value from stdin |

22
ruby/ql/test/query-tests/security/cwe-502/unsafe-deserialization/UnsafeDeserialization.rb
View File

@@ -88,15 +88,19 @@ class UsersController < ActionController::Base
object = Psych.load yaml_data
end
# BAD - user input determines which class is instantiated
def route12
klass = Module.const_get(params[:class])
object = klass.new
end
def stdin
object = YAML.load $stdin.read
# BAD - user input determines which class is instantiated
def route13
klass = ActiveJob::Serializers.deserialize(params[:class])
object = klass.new
# STDIN
object = YAML.load STDIN.gets
# ARGF
object = YAML.load ARGF.read
# Kernel.gets
object = YAML.load gets
# Kernel.readlines
object = YAML.load readlines
end
end