Skip to content

Commit 5948126

Browse files
alexrfordalexrford
authored
Merge pull request #10746 from alexrford/ruby/activejob-deserialize
Ruby: Add `ActiveJob::Serializers.deserialize` as a code execution sink
2 parents 3a1a94b + 0536d4b commit 5948126

File tree

8 files changed

+175
-86
lines changed

8 files changed

+175
-86
lines changed

‎ruby/ql/lib/change-notes/2022-10-09-activejob-serializers-deserialize.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* `ActiveJob::Serializers.deserialize` is considered to be a code execution sink.

‎ruby/ql/lib/codeql/ruby/Frameworks.qll‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
private import codeql.ruby.frameworks.Core
66
private import codeql.ruby.frameworks.ActionCable
77
private import codeql.ruby.frameworks.ActionController
8+
private import codeql.ruby.frameworks.ActiveJob
89
private import codeql.ruby.frameworks.ActionMailer
910
private import codeql.ruby.frameworks.ActiveRecord
1011
private import codeql.ruby.frameworks.ActiveResource

‎ruby/ql/lib/codeql/ruby/frameworks/ActiveJob.qll‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Modeling for `ActiveJob`, a framweork for declaring and enqueueing jobs that
3+
* ships with Rails.
4+
* https://rubygems.org/gems/activejob
5+
*/
6+
7+
private import codeql.ruby.ApiGraphs
8+
private import codeql.ruby.Concepts
9+
private import codeql.ruby.DataFlow
10+
11+
/** Modeling for `ActiveJob`. */
12+
module ActiveJob {
13+
/**
14+
* `ActiveJob::Serializers`
15+
*/
16+
module Serializers {
17+
/**
18+
* A call to `ActiveJob::Serializers.deserialize`, which interprets part of
19+
* its argument as a Ruby constant.
20+
*/
21+
class DeserializeCall extends DataFlow::CallNode, CodeExecution::Range {
22+
DeserializeCall() {
23+
this =
24+
API::getTopLevelMember("ActiveJob").getMember("Serializers").getAMethodCall("deserialize")
25+
}
26+
27+
override DataFlow::Node getCode() { result = this.getArgument(0) }
28+
}
29+
}
30+
}

‎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-094/CodeInjection.expected‎

Lines changed: 28 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,32 @@
11
edges
2-
| CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:3:12:3:24 | ...[...] : |
3-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:6:10:6:13 | code |
4-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:18:20:18:23 | code |
5-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:21:21:21:24 | code |
6-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:27:15:27:18 | code |
7-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:30:19:30:22 | code |
8-
| CodeInjection.rb:3:12:3:24 | ...[...] : | CodeInjection.rb:36:24:36:27 | code : |
9-
| CodeInjection.rb:36:24:36:27 | code : | CodeInjection.rb:36:10:36:28 | call to escape |
2+
| CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:5:12:5:24 | ...[...] : |
3+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:8:10:8:13 | code |
4+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:20:20:20:23 | code |
5+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:23:21:23:24 | code |
6+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:29:15:29:18 | code |
7+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:32:19:32:22 | code |
8+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:38:24:38:27 | code : |
9+
| CodeInjection.rb:5:12:5:24 | ...[...] : | CodeInjection.rb:41:40:41:43 | code |
10+
| CodeInjection.rb:38:24:38:27 | code : | CodeInjection.rb:38:10:38:28 | call to escape |
1011
nodes
11-
| CodeInjection.rb:3:12:3:17 | call to params : | semmle.label | call to params : |
12-
| CodeInjection.rb:3:12:3:24 | ...[...] : | semmle.label | ...[...] : |
13-
| CodeInjection.rb:6:10:6:13 | code | semmle.label | code |
14-
| CodeInjection.rb:9:10:9:15 | call to params | semmle.label | call to params |
15-
| CodeInjection.rb:18:20:18:23 | code | semmle.label | code |
16-
| CodeInjection.rb:21:21:21:24 | code | semmle.label | code |
17-
| CodeInjection.rb:27:15:27:18 | code | semmle.label | code |
18-
| CodeInjection.rb:30:19:30:22 | code | semmle.label | code |
19-
| CodeInjection.rb:36:10:36:28 | call to escape | semmle.label | call to escape |
20-
| CodeInjection.rb:36:24:36:27 | code : | semmle.label | code : |
12+
| CodeInjection.rb:5:12:5:17 | call to params : | semmle.label | call to params : |
13+
| CodeInjection.rb:5:12:5:24 | ...[...] : | semmle.label | ...[...] : |
14+
| CodeInjection.rb:8:10:8:13 | code | semmle.label | code |
15+
| CodeInjection.rb:11:10:11:15 | call to params | semmle.label | call to params |
16+
| CodeInjection.rb:20:20:20:23 | code | semmle.label | code |
17+
| CodeInjection.rb:23:21:23:24 | code | semmle.label | code |
18+
| CodeInjection.rb:29:15:29:18 | code | semmle.label | code |
19+
| CodeInjection.rb:32:19:32:22 | code | semmle.label | code |
20+
| CodeInjection.rb:38:10:38:28 | call to escape | semmle.label | call to escape |
21+
| CodeInjection.rb:38:24:38:27 | code : | semmle.label | code : |
22+
| CodeInjection.rb:41:40:41:43 | code | semmle.label | code |
2123
subpaths
2224
#select
23-
| 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 a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
24-
| 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 a $@. | CodeInjection.rb:9:10:9:15 | call to params | user-provided value |
25-
| CodeInjection.rb:18:20:18:23 | code | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:18:20:18:23 | code | This code execution depends on a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
26-
| CodeInjection.rb:21:21:21:24 | code | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:21:21:21:24 | code | This code execution depends on a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
27-
| CodeInjection.rb:27:15:27:18 | code | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:27:15:27:18 | code | This code execution depends on a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
28-
| CodeInjection.rb:30:19:30:22 | code | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:30:19:30:22 | code | This code execution depends on a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
29-
| CodeInjection.rb:36:10:36:28 | call to escape | CodeInjection.rb:3:12:3:17 | call to params : | CodeInjection.rb:36:10:36:28 | call to escape | This code execution depends on a $@. | CodeInjection.rb:3:12:3:17 | call to params | user-provided value |
25+
| 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 |
26+
| CodeInjection.rb:11:10:11:15 | call to params | CodeInjection.rb:11:10:11:15 | call to params | CodeInjection.rb:11:10:11:15 | call to params | This code execution depends on a $@. | CodeInjection.rb:11:10:11:15 | call to params | user-provided value |
27+
| CodeInjection.rb:20:20:20:23 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:20:20:20:23 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
28+
| CodeInjection.rb:23:21:23:24 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:23:21:23:24 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
29+
| CodeInjection.rb:29:15:29:18 | code | CodeInjection.rb:5:12:5:17 | call to params : | CodeInjection.rb:29:15:29:18 | code | This code execution depends on a $@. | CodeInjection.rb:5:12:5:17 | call to params | user-provided value |
30+
| 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 |
31+
| 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 |
32+
| 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 |

‎ruby/ql/test/query-tests/security/cwe-094/CodeInjection.rb‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
require 'active_job'
2+
13
class UsersController < ActionController::Base
24
def create
35
code = params[:code]
@@ -22,18 +24,21 @@ def create
2224

2325
# GOOD
2426
Bar.class_eval(code)
25-
27+
2628
# BAD
2729
const_get(code)
28-
30+
2931
# BAD
3032
Foo.const_get(code)
31-
33+
3234
# GOOD
3335
Bar.const_get(code)
3436

3537
# BAD
3638
eval(Regexp.escape(code))
39+
40+
# BAD
41+
ActiveJob::Serializers.deserialize(code)
3742
end
3843

3944
def update
@@ -62,8 +67,8 @@ class Bar
6267
def self.class_eval(x)
6368
true
6469
end
65-
70+
6671
def self.const_get(x)
6772
true
6873
end
69-
end
74+
end

0 commit comments

Comments
 (0)