mirror of
https://github.com/github/codeql.git
synced 2026-04-25 08:45:14 +02:00
Merge pull request #311 from github/nickrolfe/oj
Consider Oj.load a sink for unsafe deserialization
This commit is contained in:
Nick Rolfe
@@ -67,6 +67,113 @@ module UnsafeDeserialization {
|
||||
}
|
||||
}
|
||||
|
||||
private string getAKnownOjModeName(boolean isSafe) {
|
||||
result = ["compat", "custom", "json", "null", "rails", "strict", "wab"] and isSafe = true
|
||||
or
|
||||
result = "object" and isSafe = false
|
||||
}
|
||||
|
||||
private predicate isOjModePair(Pair p, string modeValue) {
|
||||
p.getKey().getValueText() = "mode" and
|
||||
exists(DataFlow::LocalSourceNode symbolLiteral, DataFlow::Node value |
|
||||
symbolLiteral.asExpr().getExpr().(SymbolLiteral).getValueText() = modeValue and
|
||||
symbolLiteral.flowsTo(value) and
|
||||
value.asExpr().getExpr() = p.getValue()
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* A node representing a hash that contains the key `:mode`.
|
||||
*/
|
||||
private class OjOptionsHashWithModeKey extends DataFlow::Node {
|
||||
private string modeValue;
|
||||
|
||||
OjOptionsHashWithModeKey() {
|
||||
exists(DataFlow::LocalSourceNode options |
|
||||
options.flowsTo(this) and
|
||||
isOjModePair(options.asExpr().getExpr().(HashLiteral).getAKeyValuePair(), modeValue)
|
||||
)
|
||||
}
|
||||
|
||||
/**
|
||||
* Holds if this hash node contains a `:mode` key whose value is one known
|
||||
* to be `isSafe` with untrusted data.
|
||||
*/
|
||||
predicate hasKnownMode(boolean isSafe) { modeValue = getAKnownOjModeName(isSafe) }
|
||||
|
||||
/**
|
||||
* Holds if this hash node contains a `:mode` key whose value is one of the
|
||||
* `Oj` modes known to be safe to use with untrusted data.
|
||||
*/
|
||||
predicate hasSafeMode() { this.hasKnownMode(true) }
|
||||
}
|
||||
|
||||
/**
|
||||
* A call node that sets `Oj.default_options`.
|
||||
*
|
||||
* ```rb
|
||||
* Oj.default_options = { allow_blank: true, mode: :compat }
|
||||
* ```
|
||||
*/
|
||||
private class SetOjDefaultOptionsCall extends DataFlow::CallNode {
|
||||
SetOjDefaultOptionsCall() {
|
||||
this = API::getTopLevelMember("Oj").getAMethodCall("default_options=")
|
||||
}
|
||||
|
||||
/**
|
||||
* Gets the value being assigned to `Oj.default_options`.
|
||||
*/
|
||||
DataFlow::Node getValue() {
|
||||
result.asExpr() =
|
||||
this.getArgument(0).asExpr().(CfgNodes::ExprNodes::AssignExprCfgNode).getRhs()
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A call to `Oj.load`.
|
||||
*/
|
||||
private class OjLoadCall extends DataFlow::CallNode {
|
||||
OjLoadCall() { this = API::getTopLevelMember("Oj").getAMethodCall("load") }
|
||||
|
||||
/**
|
||||
* Holds if this call to `Oj.load` includes an explicit options hash
|
||||
* argument that sets the mode to one that is known to be `isSafe`.
|
||||
*/
|
||||
predicate hasExplicitKnownMode(boolean isSafe) {
|
||||
exists(DataFlow::Node arg, int i | i >= 1 and arg = this.getArgument(i) |
|
||||
arg.(OjOptionsHashWithModeKey).hasKnownMode(isSafe)
|
||||
or
|
||||
isOjModePair(arg.asExpr().getExpr(), getAKnownOjModeName(isSafe))
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* An argument in a call to `Oj.load` where the mode is `:object` (which is
|
||||
* the default), considered a sink for unsafe deserialization.
|
||||
*/
|
||||
class UnsafeOjLoadArgument extends Sink {
|
||||
UnsafeOjLoadArgument() {
|
||||
exists(OjLoadCall ojLoad |
|
||||
this = ojLoad.getArgument(0) and
|
||||
// Exclude calls that explicitly pass a safe mode option.
|
||||
not ojLoad.hasExplicitKnownMode(true) and
|
||||
(
|
||||
// Sinks to include:
|
||||
// - Calls with an explicit, unsafe mode option.
|
||||
ojLoad.hasExplicitKnownMode(false)
|
||||
or
|
||||
// - Calls with no explicit mode option, unless there exists a call
|
||||
// anywhere to set the default options to a known safe mode.
|
||||
not ojLoad.hasExplicitKnownMode(_) and
|
||||
not exists(SetOjDefaultOptionsCall setOpts |
|
||||
setOpts.getValue().(OjOptionsHashWithModeKey).hasSafeMode()
|
||||
)
|
||||
)
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* `Base64.decode64` propagates taint from its argument to its return value.
|
||||
*/
|
||||
|
||||
@@ -21,15 +21,18 @@ deserialization of arbitrary objects.
|
||||
|
||||
<example>
|
||||
<p>
|
||||
The following example calls the <code>Marshal.load</code>, <code>JSON.load</code>, and
|
||||
<code>YAML.load</code> methods on data from an HTTP request. Since these methods
|
||||
are capable of deserializing to arbitrary objects, this is inherently unsafe.
|
||||
The following example calls the <code>Marshal.load</code>,
|
||||
<code>JSON.load</code>, <code>YAML.load</code>, and <code>Oj.load</code> methods
|
||||
on data from an HTTP request. Since these methods are capable of deserializing
|
||||
to arbitrary objects, this is inherently unsafe.
|
||||
</p>
|
||||
<sample src="examples/UnsafeDeserializationBad.rb"/>
|
||||
<p>
|
||||
Using <code>JSON.parse</code> and <code>YAML.safe_load</code> instead, as in the
|
||||
following example, removes the vulnerability. Note that there is no safe way to
|
||||
deserialize untrusted data using <code>Marshal</code>.
|
||||
following example, removes the vulnerability. Similarly, calling
|
||||
<code>Oj.load</code> with any mode other than <code>:object</code> is safe, as
|
||||
is calling <code>Oj.safe_load</code>. Note that there is no safe way to deserialize
|
||||
untrusted data using <code>Marshal</code>.
|
||||
</p>
|
||||
<sample src="examples/UnsafeDeserializationGood.rb"/>
|
||||
</example>
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
require 'json'
|
||||
require 'yaml'
|
||||
require 'oj'
|
||||
|
||||
class UserController < ActionController::Base
|
||||
def marshal_example
|
||||
@@ -17,4 +18,9 @@ class UserController < ActionController::Base
|
||||
object = YAML.load params[:yaml]
|
||||
# ...
|
||||
end
|
||||
|
||||
def oj_example
|
||||
object = Oj.load params[:json]
|
||||
# ...
|
||||
end
|
||||
end
|
||||
@@ -10,4 +10,11 @@ class UserController < ActionController::Base
|
||||
object = YAML.safe_load params[:yaml]
|
||||
# ...
|
||||
end
|
||||
|
||||
def safe_oj_example
|
||||
object = Oj.load params[:yaml], { mode: :strict }
|
||||
# or
|
||||
object = Oj.safe_load params[:yaml]
|
||||
# ...
|
||||
end
|
||||
end
|
||||
@@ -1,24 +0,0 @@
|
||||
edges
|
||||
| UnsafeDeserialization.rb:8:39:8:44 | call to params : | UnsafeDeserialization.rb:9:27:9:41 | serialized_data |
|
||||
| UnsafeDeserialization.rb:14:39:14:44 | call to params : | UnsafeDeserialization.rb:15:30:15:44 | serialized_data |
|
||||
| UnsafeDeserialization.rb:20:17:20:22 | call to params : | UnsafeDeserialization.rb:21:24:21:32 | json_data |
|
||||
| UnsafeDeserialization.rb:26:17:26:22 | call to params : | UnsafeDeserialization.rb:27:27:27:35 | json_data |
|
||||
| UnsafeDeserialization.rb:38:17:38:22 | call to params : | UnsafeDeserialization.rb:39:24:39:32 | yaml_data |
|
||||
nodes
|
||||
| UnsafeDeserialization.rb:8:39:8:44 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:9:27:9:41 | serialized_data | semmle.label | serialized_data |
|
||||
| UnsafeDeserialization.rb:14:39:14:44 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:15:30:15:44 | serialized_data | semmle.label | serialized_data |
|
||||
| UnsafeDeserialization.rb:20:17:20:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:21:24:21:32 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:26:17:26:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:27:27:27:35 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:38:17:38:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:39:24:39:32 | yaml_data | semmle.label | yaml_data |
|
||||
subpaths
|
||||
#select
|
||||
| UnsafeDeserialization.rb:9:27:9:41 | serialized_data | UnsafeDeserialization.rb:8:39:8:44 | call to params : | UnsafeDeserialization.rb:9:27:9:41 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:8:39:8:44 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:15:30:15:44 | serialized_data | UnsafeDeserialization.rb:14:39:14:44 | call to params : | UnsafeDeserialization.rb:15:30:15:44 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:14:39:14:44 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:21:24:21:32 | json_data | UnsafeDeserialization.rb:20:17:20:22 | call to params : | UnsafeDeserialization.rb:21:24:21:32 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:20:17:20:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:27:27:27:35 | json_data | UnsafeDeserialization.rb:26:17:26:22 | call to params : | UnsafeDeserialization.rb:27:27:27:35 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:26:17:26:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:39:24:39:32 | yaml_data | UnsafeDeserialization.rb:38:17:38:22 | call to params : | UnsafeDeserialization.rb:39:24:39:32 | yaml_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:38:17:38:22 | call to params | user input |
|
||||
@@ -1,47 +0,0 @@
|
||||
require "base64"
|
||||
require "json"
|
||||
require "yaml"
|
||||
|
||||
class UsersController < ActionController::Base
|
||||
# BAD
|
||||
def route0
|
||||
serialized_data = Base64.decode64 params[:key]
|
||||
object = Marshal.load serialized_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route1
|
||||
serialized_data = Base64.decode64 params[:key]
|
||||
object = Marshal.restore serialized_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route2
|
||||
json_data = params[:key]
|
||||
object = JSON.load json_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route3
|
||||
json_data = params[:key]
|
||||
object = JSON.restore json_data
|
||||
end
|
||||
|
||||
# GOOD - JSON.parse is safe to use on untrusted data
|
||||
def route4
|
||||
json_data = params[:key]
|
||||
object = JSON.parse json_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route5
|
||||
yaml_data = params[:key]
|
||||
object = YAML.load yaml_data
|
||||
end
|
||||
|
||||
# GOOD
|
||||
def route6
|
||||
yaml_data = params[:key]
|
||||
object = YAML.safe_load yaml_data
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,16 @@
|
||||
require "oj"
|
||||
|
||||
class UsersController < ActionController::Base
|
||||
# GOOD - Oj.load is safe when any mode other than :object is set globally
|
||||
def route0
|
||||
json_data = params[:key]
|
||||
object = Oj.load json_data
|
||||
end
|
||||
|
||||
# BAD - the safe mode set globally is overridden with an unsafe mode passed as
|
||||
# a call argument
|
||||
def route1
|
||||
json_data = params[:key]
|
||||
object = Oj.load json_data, mode: :object
|
||||
end
|
||||
end
|
||||
@@ -0,0 +1,5 @@
|
||||
require "oj"
|
||||
|
||||
# Set the default mode for the Oj library to use the :compat mode, which makes
|
||||
# Oj.load safe for untrusted data.
|
||||
Oj.default_options = { :mode => :compat }
|
||||
@@ -0,0 +1,8 @@
|
||||
edges
|
||||
| OjGlobalOptions.rb:13:17:13:22 | call to params : | OjGlobalOptions.rb:14:22:14:30 | json_data |
|
||||
nodes
|
||||
| OjGlobalOptions.rb:13:17:13:22 | call to params : | semmle.label | call to params : |
|
||||
| OjGlobalOptions.rb:14:22:14:30 | json_data | semmle.label | json_data |
|
||||
subpaths
|
||||
#select
|
||||
| OjGlobalOptions.rb:14:22:14:30 | json_data | OjGlobalOptions.rb:13:17:13:22 | call to params : | OjGlobalOptions.rb:14:22:14:30 | json_data | Unsafe deserialization of $@. | OjGlobalOptions.rb:13:17:13:22 | call to params | user input |
|
||||
@@ -0,0 +1,35 @@
|
||||
edges
|
||||
| UnsafeDeserialization.rb:9:39:9:44 | call to params : | UnsafeDeserialization.rb:10:27:10:41 | serialized_data |
|
||||
| UnsafeDeserialization.rb:15:39:15:44 | call to params : | UnsafeDeserialization.rb:16:30:16:44 | serialized_data |
|
||||
| UnsafeDeserialization.rb:21:17:21:22 | call to params : | UnsafeDeserialization.rb:22:24:22:32 | json_data |
|
||||
| UnsafeDeserialization.rb:27:17:27:22 | call to params : | UnsafeDeserialization.rb:28:27:28:35 | json_data |
|
||||
| UnsafeDeserialization.rb:39:17:39:22 | call to params : | UnsafeDeserialization.rb:40:24:40:32 | yaml_data |
|
||||
| UnsafeDeserialization.rb:51:17:51:22 | call to params : | UnsafeDeserialization.rb:52:22:52:30 | json_data |
|
||||
| UnsafeDeserialization.rb:51:17:51:22 | call to params : | UnsafeDeserialization.rb:53:22:53:30 | json_data |
|
||||
| UnsafeDeserialization.rb:58:17:58:22 | call to params : | UnsafeDeserialization.rb:68:23:68:31 | json_data |
|
||||
nodes
|
||||
| UnsafeDeserialization.rb:9:39:9:44 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:10:27:10:41 | serialized_data | semmle.label | serialized_data |
|
||||
| UnsafeDeserialization.rb:15:39:15:44 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:16:30:16:44 | serialized_data | semmle.label | serialized_data |
|
||||
| UnsafeDeserialization.rb:21:17:21:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:22:24:22:32 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:27:17:27:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:28:27:28:35 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:39:17:39:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:40:24:40:32 | yaml_data | semmle.label | yaml_data |
|
||||
| UnsafeDeserialization.rb:51:17:51:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:52:22:52:30 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:53:22:53:30 | json_data | semmle.label | json_data |
|
||||
| UnsafeDeserialization.rb:58:17:58:22 | call to params : | semmle.label | call to params : |
|
||||
| UnsafeDeserialization.rb:68:23:68:31 | json_data | semmle.label | json_data |
|
||||
subpaths
|
||||
#select
|
||||
| UnsafeDeserialization.rb:10:27:10:41 | serialized_data | UnsafeDeserialization.rb:9:39:9:44 | call to params : | UnsafeDeserialization.rb:10:27:10:41 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:9:39:9:44 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:16:30:16:44 | serialized_data | UnsafeDeserialization.rb:15:39:15:44 | call to params : | UnsafeDeserialization.rb:16:30:16:44 | serialized_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:15:39:15:44 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:22:24:22:32 | json_data | UnsafeDeserialization.rb:21:17:21:22 | call to params : | UnsafeDeserialization.rb:22:24:22:32 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:21:17:21:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:28:27:28:35 | json_data | UnsafeDeserialization.rb:27:17:27:22 | call to params : | UnsafeDeserialization.rb:28:27:28:35 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:27:17:27:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:40:24:40:32 | yaml_data | UnsafeDeserialization.rb:39:17:39:22 | call to params : | UnsafeDeserialization.rb:40:24:40:32 | yaml_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:39:17:39:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:52:22:52:30 | json_data | UnsafeDeserialization.rb:51:17:51:22 | call to params : | UnsafeDeserialization.rb:52:22:52:30 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:51:17:51:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:53:22:53:30 | json_data | UnsafeDeserialization.rb:51:17:51:22 | call to params : | UnsafeDeserialization.rb:53:22:53:30 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:51:17:51:22 | call to params | user input |
|
||||
| UnsafeDeserialization.rb:68:23:68:31 | json_data | UnsafeDeserialization.rb:58:17:58:22 | call to params : | UnsafeDeserialization.rb:68:23:68:31 | json_data | Unsafe deserialization of $@. | UnsafeDeserialization.rb:58:17:58:22 | call to params | user input |
|
||||
@@ -0,0 +1 @@
|
||||
queries/security/cwe-502/UnsafeDeserialization.ql
|
||||
@@ -0,0 +1,76 @@
|
||||
require "base64"
|
||||
require "json"
|
||||
require "oj"
|
||||
require "yaml"
|
||||
|
||||
class UsersController < ActionController::Base
|
||||
# BAD
|
||||
def route0
|
||||
serialized_data = Base64.decode64 params[:key]
|
||||
object = Marshal.load serialized_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route1
|
||||
serialized_data = Base64.decode64 params[:key]
|
||||
object = Marshal.restore serialized_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route2
|
||||
json_data = params[:key]
|
||||
object = JSON.load json_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route3
|
||||
json_data = params[:key]
|
||||
object = JSON.restore json_data
|
||||
end
|
||||
|
||||
# GOOD - JSON.parse is safe to use on untrusted data
|
||||
def route4
|
||||
json_data = params[:key]
|
||||
object = JSON.parse json_data
|
||||
end
|
||||
|
||||
# BAD
|
||||
def route5
|
||||
yaml_data = params[:key]
|
||||
object = YAML.load yaml_data
|
||||
end
|
||||
|
||||
# GOOD
|
||||
def route6
|
||||
yaml_data = params[:key]
|
||||
object = YAML.safe_load yaml_data
|
||||
end
|
||||
|
||||
# BAD - Oj.load is unsafe in its default :object mode
|
||||
def route7
|
||||
json_data = params[:key]
|
||||
object = Oj.load json_data
|
||||
object = Oj.load json_data, mode: :object
|
||||
end
|
||||
|
||||
# GOOD - Oj.load is safe in any other mode
|
||||
def route8
|
||||
json_data = params[:key]
|
||||
# Test the different ways the options hash can be passed
|
||||
options = { allow_blank: true, mode: :rails }
|
||||
object1 = Oj.load json_data, options
|
||||
object2 = Oj.load json_data, mode: :strict
|
||||
object3 = Oj.load json_data, :allow_blank => true, :mode => :compat
|
||||
|
||||
# TODO: false positive; we aren't detecting flow from `:json` to the call argument.
|
||||
more_options = { allow_blank: true }
|
||||
more_options[:mode] = :json
|
||||
object4 = Oj.load json_data, more_options
|
||||
end
|
||||
|
||||
# GOOD
|
||||
def route9
|
||||
json_data = params[:key]
|
||||
object = Oj.safe_load json_data
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user