Skip to content

Commit 3d08a29

Browse files
committed
Ruby: add rb/unsafe-deserialization sinks for const_get args
1 parent a3f096a commit 3d08a29

File tree

3 files changed

+45
-0
lines changed

3 files changed

+45
-0
lines changed

ruby/ql/lib/codeql/ruby/security/UnsafeDeserializationCustomizations.qll

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ private import codeql.ruby.ApiGraphs
88
private import codeql.ruby.CFG
99
private import codeql.ruby.DataFlow
1010
private import codeql.ruby.dataflow.RemoteFlowSources
11+
private import codeql.ruby.frameworks.ActiveJob
12+
private import codeql.ruby.frameworks.core.Module
1113

1214
module UnsafeDeserialization {
1315
/**
@@ -199,4 +201,27 @@ module UnsafeDeserialization {
199201
toNode = callNode
200202
)
201203
}
204+
205+
/**
206+
* A argument in a call to `Module.const_get`, considered as a sink for unsafe
207+
* deserialization.
208+
*
209+
* Calls to `Module.const_get` can return arbitrary classes which can then be
210+
* instantiated.
211+
*/
212+
class ConstGetCallArgument extends Sink {
213+
ConstGetCallArgument() { this = any(Module::ModuleConstGetCallCodeExecution c).getCode() }
214+
}
215+
216+
/**
217+
* A argument in a call to `ActiveJob::Serializers.deserialize`, considered as
218+
* a sink for unsafe deserialization.
219+
*
220+
* This is roughly equivalent to a call to `Module.const_get`.
221+
*/
222+
class ActiveJobSerializersDeserializeArgument extends Sink {
223+
ActiveJobSerializersDeserializeArgument() {
224+
this = any(ActiveJob::Serializers::DeserializeCall c).getCode()
225+
}
226+
}
202227
}

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ edges
1818
| UnsafeDeserialization.rb:81:11:81:22 | ...[...] : | UnsafeDeserialization.rb:82:34:82:36 | xml |
1919
| UnsafeDeserialization.rb:87:17:87:22 | call to params : | UnsafeDeserialization.rb:87:17:87:28 | ...[...] : |
2020
| UnsafeDeserialization.rb:87:17:87:28 | ...[...] : | UnsafeDeserialization.rb:88:25:88:33 | yaml_data |
21+
| UnsafeDeserialization.rb:93:30:93:35 | call to params : | UnsafeDeserialization.rb:93:30:93:43 | ...[...] |
22+
| UnsafeDeserialization.rb:99:48:99:53 | call to params : | UnsafeDeserialization.rb:99:48:99:61 | ...[...] |
2123
nodes
2224
| UnsafeDeserialization.rb:10:39:10:44 | call to params : | semmle.label | call to params : |
2325
| UnsafeDeserialization.rb:10:39:10:50 | ...[...] : | semmle.label | ...[...] : |
@@ -47,6 +49,10 @@ nodes
4749
| UnsafeDeserialization.rb:87:17:87:22 | call to params : | semmle.label | call to params : |
4850
| UnsafeDeserialization.rb:87:17:87:28 | ...[...] : | semmle.label | ...[...] : |
4951
| UnsafeDeserialization.rb:88:25:88:33 | yaml_data | semmle.label | yaml_data |
52+
| UnsafeDeserialization.rb:93:30:93:35 | call to params : | semmle.label | call to params : |
53+
| UnsafeDeserialization.rb:93:30:93:43 | ...[...] | semmle.label | ...[...] |
54+
| UnsafeDeserialization.rb:99:48:99:53 | call to params : | semmle.label | call to params : |
55+
| UnsafeDeserialization.rb:99:48:99:61 | ...[...] | semmle.label | ...[...] |
5056
subpaths
5157
#select
5258
| 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 |
@@ -59,3 +65,5 @@ subpaths
5965
| 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 |
6066
| 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 |
6167
| 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 |
68+
| 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 |
69+
| 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 |

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,4 +87,16 @@ def route11
8787
yaml_data = params[:key]
8888
object = Psych.load yaml_data
8989
end
90+
91+
# BAD - user input determines which class is instantiated
92+
def route12
93+
klass = Module.const_get(params[:class])
94+
object = klass.new
95+
end
96+
97+
# BAD - user input determines which class is instantiated
98+
def route13
99+
klass = ActiveJob::Serializers.deserialize(params[:class])
100+
object = klass.new
101+
end
90102
end

0 commit comments

Comments
 (0)