Skip to content

Commit adceb0a

Browse files
committed
Add query rb/unsafe-deserialization
1 parent a62aa2b commit adceb0a

File tree

6 files changed

+197
-0
lines changed

6 files changed

+197
-0
lines changed
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/**
2+
* Provides default sources, sinks and sanitizers for reasoning about unsafe
3+
* deserialization, as well as extension points for adding your own.
4+
*/
5+
6+
private import ruby
7+
private import codeql.ruby.ApiGraphs
8+
private import codeql.ruby.CFG
9+
private import codeql.ruby.DataFlow
10+
private import codeql.ruby.dataflow.RemoteFlowSources
11+
12+
module UnsafeDeserialization {
13+
/**
14+
* A data flow source for unsafe deserialization vulnerabilities.
15+
*/
16+
abstract class Source extends DataFlow::Node { }
17+
18+
/**
19+
* A data flow sink for unsafe deserialization vulnerabilities.
20+
*/
21+
abstract class Sink extends DataFlow::Node { }
22+
23+
/**
24+
* A sanitizer for unsafe deserialization vulnerabilities.
25+
*/
26+
abstract class Sanitizer extends DataFlow::Node { }
27+
28+
/** A source of remote user input, considered as a flow source for unsafe deserialization. */
29+
class RemoteFlowSourceAsSource extends Source {
30+
RemoteFlowSourceAsSource() { this instanceof RemoteFlowSource }
31+
}
32+
33+
/**
34+
* An argument in a call to `Marshal.load` or `Marshal.restore`, considered a
35+
* sink for unsafe deserialization.
36+
*/
37+
class MarshalLoadOrRestoreArgument extends Sink {
38+
MarshalLoadOrRestoreArgument() {
39+
this = API::getTopLevelMember("Marshal").getAMethodCall(["load", "restore"]).getArgument(0)
40+
}
41+
}
42+
43+
/**
44+
* An argument in a call to `YAML.load`, considered a sink for unsafe
45+
* deserialization.
46+
*/
47+
class YamlLoadArgument extends Sink {
48+
YamlLoadArgument() {
49+
this = API::getTopLevelMember("YAML").getAMethodCall("load").getArgument(0)
50+
}
51+
}
52+
53+
/**
54+
* An argument in a call to `JSON.load` or `JSON.restore`, considered a sink
55+
* for unsafe deserialization.
56+
*/
57+
class JsonLoadArgument extends Sink {
58+
JsonLoadArgument() {
59+
this = API::getTopLevelMember("JSON").getAMethodCall(["load", "restore"]).getArgument(0)
60+
}
61+
}
62+
}
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/**
2+
* Provides a taint-tracking configuration for reasoning about unsafe deserialization.
3+
*
4+
* Note, for performance reasons: only import this file if
5+
* `UnsafeDeserialization::Configuration` is needed, otherwise
6+
* `UnsafeDeserializationCustomizations` should be imported instead.
7+
*/
8+
9+
private import ruby
10+
private import codeql.ruby.DataFlow
11+
private import codeql.ruby.TaintTracking
12+
import UnsafeDeserializationCustomizations
13+
14+
/**
15+
* A taint-tracking configuration for reasoning about unsafe deserialization.
16+
*/
17+
class Configuration extends TaintTracking::Configuration {
18+
Configuration() { this = "UnsafeDeserialization" }
19+
20+
override predicate isSource(DataFlow::Node source) {
21+
source instanceof UnsafeDeserialization::Source
22+
}
23+
24+
override predicate isSink(DataFlow::Node sink) { sink instanceof UnsafeDeserialization::Sink }
25+
26+
override predicate isSanitizer(DataFlow::Node node) {
27+
super.isSanitizer(node) or
28+
node instanceof UnsafeDeserialization::Sanitizer
29+
}
30+
}
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
<!DOCTYPE qhelp PUBLIC "-//Semmle//qhelp//EN" "qhelp.dtd">
2+
<qhelp>
3+
4+
<overview>
5+
<p>
6+
Deserializing untrusted data using any method that allows the construction of
7+
arbitrary objects is easily exploitable and, in many cases, allows an attacker
8+
to execute arbitrary code.
9+
</p>
10+
</overview>
11+
12+
<recommendation>
13+
<p>
14+
Avoid deserialization of untrusted data if possible. If the architecture permits
15+
it, use serialization formats that cannot represent arbitarary objects. For
16+
libraries that support it, such as the Ruby standard library's <code>JSON</code>
17+
module, ensure that the parser is configured to disable
18+
deserialization of arbitrary objects.
19+
</p>
20+
</recommendation>
21+
22+
<example>
23+
<p>
24+
The following example calls the <code>Marshal.load</code>, <code>JSON.load</code>, and
25+
<code>YAML.load</code> methods on data from an HTTP request. Since these methods
26+
are capable of deserializing to arbitrary objects, this is inherently unsafe.
27+
</p>
28+
<sample src="examples/UnsafeDeserializationBad.rb"/>
29+
<p>
30+
Using <code>YAML.parse</code> instead, with the default options, removes the
31+
vulnerability.
32+
</p>
33+
<sample src="examples/UnsafeDeserializationGood.rb"/>
34+
</example>
35+
36+
<references>
37+
38+
<li>
39+
OWASP vulnerability description:
40+
<a href="https://www.owasp.org/index.php/Deserialization_of_untrusted_data">deserialization of untrusted data</a>.
41+
</li>
42+
<li>
43+
Ruby documentation: <a href="https://docs.ruby-lang.org/en/3.0.0/doc/security_rdoc.html">guidance on deserializing objects safely</a>.
44+
</li>
45+
<li>
46+
Ruby documentation: <a href="https://ruby-doc.org/core-3.0.2/Marshal.html#module-Marshal-label-Security+considerations">security guidance on the Marshal library</a>.
47+
</li>
48+
<li>
49+
Ruby documentation: <a href="https://ruby-doc.org/stdlib-3.0.2/libdoc/json/rdoc/JSON.html#method-i-load">security guidance on JSON.load</a>.
50+
</li>
51+
<li>
52+
Ruby documentation: <a href="https://ruby-doc.org/stdlib-3.0.2/libdoc/yaml/rdoc/YAML.html#module-YAML-label-Security">security guidance on the YAML library</a>.
53+
</li>
54+
</references>
55+
56+
</qhelp>
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
/**
2+
* @name Deserialization of user-controlled data
3+
* @description Deserializing user-controlled data may allow attackers to
4+
* execute arbitrary code.
5+
* @kind path-problem
6+
* @problem.severity warning
7+
* @security-severity 9.8
8+
* @precision high
9+
* @id rb/unsafe-deserialization
10+
* @tags security
11+
* external/cwe/cwe-502
12+
*/
13+
14+
import ruby
15+
import DataFlow::PathGraph
16+
import codeql.ruby.DataFlow
17+
import codeql.ruby.security.UnsafeDeserializationQuery
18+
19+
from Configuration cfg, DataFlow::PathNode source, DataFlow::PathNode sink
20+
where cfg.hasFlowPath(source, sink)
21+
select sink.getNode(), source, sink, "Unsafe deserialization of $@.", source.getNode(), "user input"
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
require 'json'
2+
require 'yaml'
3+
4+
class UserController < ActionController::Base
5+
def marshal_example
6+
data = Base64.decode64 params[:data]
7+
object = Marshal.load data
8+
# ...
9+
end
10+
11+
def json_example
12+
object = JSON.load params[:json]
13+
# ...
14+
end
15+
16+
def yaml_example
17+
object = YAML.load params[:yaml]
18+
# ...
19+
end
20+
end
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
require 'json'
2+
3+
class UserController < ActionController::Base
4+
def safe_json_example
5+
object = JSON.parse params[:json]
6+
# ...
7+
end
8+
end

0 commit comments

Comments
 (0)