Skip to content

Commit

Permalink
[ruby/psych] Introduce Psych.unsafe_load
Browse files Browse the repository at this point in the history
In future versions of Psych, the `load` method will be mostly the same
as the `safe_load` method.  In other words, the `load` method won't
allow arbitrary object deserialization (which can be used to escalate to
an RCE).  People that need to load *trusted* documents can use the
`unsafe_load` method.

This commit introduces the `unsafe_load` method so that people can
incrementally upgrade.  For example, if they try to upgrade to 4.0.0 and
something breaks, they can downgrade, audit callsites, change to
`safe_load` or `unsafe_load` as required, and then upgrade to 4.0.0
smoothly.

ruby/psych@cb50aa8d3f
  • Loading branch information
tenderlove authored and hsbt committed May 17, 2021
1 parent bcaa6ae commit c7c2ad5
Show file tree
Hide file tree
Showing 26 changed files with 156 additions and 123 deletions.
8 changes: 5 additions & 3 deletions ext/psych/lib/psych.rb
Expand Up @@ -271,7 +271,7 @@ module Psych
# YAML documents that are supplied via user input. Instead, please use the
# safe_load method.
#
def self.load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false, freeze: false
def self.unsafe_load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false, symbolize_names: false, freeze: false
if legacy_filename != NOT_GIVEN
warn_with_uplevel 'Passing filename with the 2nd argument of Psych.load is deprecated. Use keyword argument like Psych.load(yaml, filename: ...) instead.', uplevel: 1 if $VERBOSE
filename = legacy_filename
Expand All @@ -281,6 +281,7 @@ def self.load yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback: false,
return fallback unless result
result.to_ruby(symbolize_names: symbolize_names, freeze: freeze)
end
class << self; alias :load :unsafe_load; end

###
# Safely load the yaml string in +yaml+. By default, only the following
Expand Down Expand Up @@ -577,11 +578,12 @@ def self.load_stream yaml, legacy_filename = NOT_GIVEN, filename: nil, fallback:
# NOTE: This method *should not* be used to parse untrusted documents, such as
# YAML documents that are supplied via user input. Instead, please use the
# safe_load_file method.
def self.load_file filename, **kwargs
def self.unsafe_load_file filename, **kwargs
File.open(filename, 'r:bom|utf-8') { |f|
self.load f, filename: filename, **kwargs
self.unsafe_load f, filename: filename, **kwargs
}
end
class << self; alias :load_file :unsafe_load_file; end

###
# Safely loads the document contained in +filename+. Returns the yaml contained in
Expand Down
4 changes: 2 additions & 2 deletions ext/psych/lib/psych/versions.rb
@@ -1,8 +1,8 @@

# frozen_string_literal: true

module Psych
# The version of Psych you are using
VERSION = '3.3.1'
VERSION = '3.3.2'

if RUBY_ENGINE == 'jruby'
DEFAULT_SNAKEYAML_VERSION = '1.28'.freeze
Expand Down
30 changes: 21 additions & 9 deletions test/psych/helper.rb
Expand Up @@ -41,24 +41,30 @@ def with_default_internal(enc)
# Convert between Psych and the object to verify correct parsing and
# emitting
#
def assert_to_yaml( obj, yaml )
assert_equal( obj, Psych::load( yaml ) )
def assert_to_yaml( obj, yaml, loader = :load )
assert_equal( obj, Psych.send(loader, yaml) )
assert_equal( obj, Psych::parse( yaml ).transform )
assert_equal( obj, Psych::load( obj.to_yaml ) )
assert_equal( obj, Psych.send(loader, obj.to_yaml) )
assert_equal( obj, Psych::parse( obj.to_yaml ).transform )
assert_equal( obj, Psych::load(
assert_equal( obj, Psych.send(loader,
obj.to_yaml(
:UseVersion => true, :UseHeader => true, :SortKeys => true
)
))
rescue Psych::DisallowedClass, Psych::BadAlias
assert_to_yaml obj, yaml, :unsafe_load
end

#
# Test parser only
#
def assert_parse_only( obj, yaml )
assert_equal( obj, Psych::load( yaml ) )
assert_equal( obj, Psych::parse( yaml ).transform )
begin
assert_equal obj, Psych::load( yaml )
rescue Psych::DisallowedClass, Psych::BadAlias
assert_equal obj, Psych::unsafe_load( yaml )
end
assert_equal obj, Psych::parse( yaml ).transform
end

def assert_cycle( obj )
Expand All @@ -69,9 +75,15 @@ def assert_cycle( obj )
assert_nil Psych::load(Psych.dump(obj))
assert_nil Psych::load(obj.to_yaml)
else
assert_equal(obj, Psych.load(v.tree.yaml))
assert_equal(obj, Psych::load(Psych.dump(obj)))
assert_equal(obj, Psych::load(obj.to_yaml))
begin
assert_equal(obj, Psych.load(v.tree.yaml))
assert_equal(obj, Psych::load(Psych.dump(obj)))
assert_equal(obj, Psych::load(obj.to_yaml))
rescue Psych::DisallowedClass, Psych::BadAlias
assert_equal(obj, Psych.unsafe_load(v.tree.yaml))
assert_equal(obj, Psych::unsafe_load(Psych.dump(obj)))
assert_equal(obj, Psych::unsafe_load(obj.to_yaml))
end
end
end

Expand Down
12 changes: 6 additions & 6 deletions test/psych/test_alias_and_anchor.rb
Expand Up @@ -19,7 +19,7 @@ def test_mri_compatibility
- *id001
- *id001
EOYAML
result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each {|el| assert_same(result[0], el) }
end

Expand All @@ -33,7 +33,7 @@ def test_mri_compatibility_object_with_ivars
- *id001
EOYAML

result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each do |el|
assert_same(result[0], el)
assert_equal('test1', el.var1)
Expand All @@ -50,7 +50,7 @@ def test_mri_compatibility_substring_with_ivars
- *id001
- *id001
EOYAML
result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each do |el|
assert_same(result[0], el)
assert_equal('test', el.var1)
Expand All @@ -62,7 +62,7 @@ def test_anchor_alias_round_trip
original = [o,o,o]

yaml = Psych.dump original
result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each {|el| assert_same(result[0], el) }
end

Expand All @@ -73,7 +73,7 @@ def test_anchor_alias_round_trip_object_with_ivars
original = [o,o,o]

yaml = Psych.dump original
result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each do |el|
assert_same(result[0], el)
assert_equal('test1', el.var1)
Expand All @@ -87,7 +87,7 @@ def test_anchor_alias_round_trip_substring_with_ivars
original = [o,o,o]

yaml = Psych.dump original
result = Psych.load yaml
result = Psych.unsafe_load yaml
result.each do |el|
assert_same(result[0], el)
assert_equal('test', el.var1)
Expand Down
6 changes: 3 additions & 3 deletions test/psych/test_array.rb
Expand Up @@ -24,7 +24,7 @@ def test_enumerator
def test_another_subclass_with_attributes
y = Y.new.tap {|o| o.val = 1}
y << "foo" << "bar"
y = Psych.load Psych.dump y
y = Psych.unsafe_load Psych.dump y

assert_equal %w{foo bar}, y
assert_equal Y, y.class
Expand All @@ -42,13 +42,13 @@ def test_subclass
end

def test_subclass_with_attributes
y = Psych.load Psych.dump Y.new.tap {|o| o.val = 1}
y = Psych.unsafe_load Psych.dump Y.new.tap {|o| o.val = 1}
assert_equal Y, y.class
assert_equal 1, y.val
end

def test_backwards_with_syck
x = Psych.load "--- !seq:#{X.name} []\n\n"
x = Psych.unsafe_load "--- !seq:#{X.name} []\n\n"
assert_equal X, x.class
end

Expand Down
14 changes: 7 additions & 7 deletions test/psych/test_coder.rb
Expand Up @@ -124,7 +124,7 @@ def encode_with(coder)

def test_self_referential
x = Referential.new
copy = Psych.load Psych.dump x
copy = Psych.unsafe_load Psych.dump x
assert_equal copy, copy.a
end

Expand Down Expand Up @@ -163,23 +163,23 @@ def test_map_with_tag_and_style
end

def test_represent_map
thing = Psych.load(Psych.dump(RepresentWithMap.new))
thing = Psych.unsafe_load(Psych.dump(RepresentWithMap.new))
assert_equal({ "string" => 'a', :symbol => 'b' }, thing.map)
end

def test_represent_sequence
thing = Psych.load(Psych.dump(RepresentWithSeq.new))
thing = Psych.unsafe_load(Psych.dump(RepresentWithSeq.new))
assert_equal %w{ foo bar }, thing.seq
end

def test_represent_with_init
thing = Psych.load(Psych.dump(RepresentWithInit.new))
thing = Psych.unsafe_load(Psych.dump(RepresentWithInit.new))
assert_equal 'bar', thing.str
end

def test_represent!
assert_match(/foo/, Psych.dump(Represent.new))
assert_instance_of(Represent, Psych.load(Psych.dump(Represent.new)))
assert_instance_of(Represent, Psych.unsafe_load(Psych.dump(Represent.new)))
end

def test_scalar_coder
Expand All @@ -189,7 +189,7 @@ def test_scalar_coder

def test_load_dumped_tagging
foo = InitApi.new
bar = Psych.load(Psych.dump(foo))
bar = Psych.unsafe_load(Psych.dump(foo))
assert_equal false, bar.implicit
assert_equal "!ruby/object:Psych::TestCoder::InitApi", bar.tag
assert_equal Psych::Nodes::Mapping::BLOCK, bar.style
Expand All @@ -208,7 +208,7 @@ def test_dump_encode_with

def test_dump_init_with
foo = InitApi.new
bar = Psych.load(Psych.dump(foo))
bar = Psych.unsafe_load(Psych.dump(foo))
assert_equal foo.a, bar.a
assert_equal foo.b, bar.b
assert_nil bar.c
Expand Down
4 changes: 2 additions & 2 deletions test/psych/test_date_time.rb
Expand Up @@ -22,7 +22,7 @@ def test_non_utc
def test_timezone_offset
times = [Time.new(2017, 4, 13, 12, 0, 0, "+09:00"),
Time.new(2017, 4, 13, 12, 0, 0, "-05:00")]
cycled = Psych::load(Psych.dump times)
cycled = Psych::unsafe_load(Psych.dump times)
assert_match(/12:00:00 \+0900/, cycled.first.to_s)
assert_match(/12:00:00 -0500/, cycled.last.to_s)
end
Expand All @@ -39,7 +39,7 @@ def test_datetime_non_utc
def test_datetime_timezone_offset
times = [DateTime.new(2017, 4, 13, 12, 0, 0, "+09:00"),
DateTime.new(2017, 4, 13, 12, 0, 0, "-05:00")]
cycled = Psych::load(Psych.dump times)
cycled = Psych::unsafe_load(Psych.dump times)
assert_match(/12:00:00\+09:00/, cycled.first.to_s)
assert_match(/12:00:00-05:00/, cycled.last.to_s)
end
Expand Down
4 changes: 2 additions & 2 deletions test/psych/test_deprecated.rb
Expand Up @@ -41,7 +41,7 @@ def to_yaml opts = {}
def test_recursive_quick_emit_encode_with
qeew = QuickEmitterEncodeWith.new
hash = { :qe => qeew }
hash2 = Psych.load Psych.dump hash
hash2 = Psych.unsafe_load Psych.dump hash
qe = hash2[:qe]

assert_equal qeew.name, qe.name
Expand Down Expand Up @@ -72,7 +72,7 @@ def yaml_initialize tag, vals
# receive the yaml_initialize call.
def test_yaml_initialize_and_init_with
hash = { :yi => YamlInitAndInitWith.new }
hash2 = Psych.load Psych.dump hash
hash2 = Psych.unsafe_load Psych.dump hash
yi = hash2[:yi]

assert_equal 'TGIF!', yi.name
Expand Down
8 changes: 4 additions & 4 deletions test/psych/test_exception.rb
Expand Up @@ -33,13 +33,13 @@ def make_ex msg = 'oh no!'

def test_backtrace
err = make_ex
new_err = Psych.load(Psych.dump(err))
new_err = Psych.unsafe_load(Psych.dump(err))
assert_equal err.backtrace, new_err.backtrace
end

def test_naming_exception
err = String.xxx rescue $!
new_err = Psych.load(Psych.dump(err))
new_err = Psych.unsafe_load(Psych.dump(err))
assert_equal err.message, new_err.message
end

Expand All @@ -56,7 +56,7 @@ def test_load_takes_file

# deprecated interface
ex = assert_raise(Psych::SyntaxError) do
Psych.load '--- `', 'deprecated'
Psych.unsafe_load '--- `', 'deprecated'
end
assert_equal 'deprecated', ex.file
end
Expand Down Expand Up @@ -165,7 +165,7 @@ def test_attributes
end

def test_convert
w = Psych.load(Psych.dump(@wups))
w = Psych.unsafe_load(Psych.dump(@wups))
assert_equal @wups.message, w.message
assert_equal @wups.backtrace, w.backtrace
assert_equal 1, w.foo
Expand Down
16 changes: 8 additions & 8 deletions test/psych/test_hash.rb
Expand Up @@ -39,7 +39,7 @@ def setup
def test_hash_with_ivar
t1 = HashWithIvar.new
t1[:foo] = :bar
t2 = Psych.load(Psych.dump(t1))
t2 = Psych.unsafe_load(Psych.dump(t1))
assert_equal t1, t2
assert_cycle t1
end
Expand All @@ -54,14 +54,14 @@ def test_referenced_hash_with_ivar
def test_custom_initialized
a = [1,2,3,4,5]
t1 = HashWithCustomInit.new(a)
t2 = Psych.load(Psych.dump(t1))
t2 = Psych.unsafe_load(Psych.dump(t1))
assert_equal t1, t2
assert_cycle t1
end

def test_custom_initialize_no_ivar
t1 = HashWithCustomInitNoIvar.new(nil)
t2 = Psych.load(Psych.dump(t1))
t2 = Psych.unsafe_load(Psych.dump(t1))
assert_equal t1, t2
assert_cycle t1
end
Expand All @@ -70,25 +70,25 @@ def test_hash_subclass_with_ivars
x = X.new
x[:a] = 'b'
x.instance_variable_set :@foo, 'bar'
dup = Psych.load Psych.dump x
dup = Psych.unsafe_load Psych.dump x
assert_cycle x
assert_equal 'bar', dup.instance_variable_get(:@foo)
assert_equal X, dup.class
end

def test_load_with_class_syck_compatibility
hash = Psych.load "--- !ruby/object:Hash\n:user_id: 7\n:username: Lucas\n"
hash = Psych.unsafe_load "--- !ruby/object:Hash\n:user_id: 7\n:username: Lucas\n"
assert_equal({ user_id: 7, username: 'Lucas'}, hash)
end

def test_empty_subclass
assert_match "!ruby/hash:#{X}", Psych.dump(X.new)
x = Psych.load Psych.dump X.new
x = Psych.unsafe_load Psych.dump X.new
assert_equal X, x.class
end

def test_map
x = Psych.load "--- !map:#{X} { }\n"
x = Psych.unsafe_load "--- !map:#{X} { }\n"
assert_equal X, x.class
end

Expand All @@ -102,7 +102,7 @@ def test_cycles
end

def test_ref_append
hash = Psych.load(<<-eoyml)
hash = Psych.unsafe_load(<<-eoyml)
---
foo: &foo
hello: world
Expand Down
6 changes: 3 additions & 3 deletions test/psych/test_marshalable.rb
Expand Up @@ -6,7 +6,7 @@ module Psych
class TestMarshalable < TestCase
def test_objects_defining_marshal_dump_and_marshal_load_can_be_dumped
sd = SimpleDelegator.new(1)
loaded = Psych.load(Psych.dump(sd))
loaded = Psych.unsafe_load(Psych.dump(sd))

assert_instance_of(SimpleDelegator, loaded)
assert_equal(sd, loaded)
Expand Down Expand Up @@ -46,15 +46,15 @@ def class

def test_init_with_takes_priority_over_marshal_methods
obj = PsychCustomMarshalable.new(1)
loaded = Psych.load(Psych.dump(obj))
loaded = Psych.unsafe_load(Psych.dump(obj))

assert(PsychCustomMarshalable === loaded)
assert_equal(2, loaded.foo)
end

def test_init_symbolize_names
obj = PsychCustomMarshalable.new(1)
loaded = Psych.load(Psych.dump(obj), symbolize_names: true)
loaded = Psych.unsafe_load(Psych.dump(obj), symbolize_names: true)

assert(PsychCustomMarshalable === loaded)
assert_equal(2, loaded.foo)
Expand Down

0 comments on commit c7c2ad5

Please sign in to comment.