ruby-changes:66219
From: Aaron <ko1@a...>
Date: Mon, 17 May 2021 11:21:07 +0900 (JST)
Subject: [ruby-changes:66219] c7c2ad5749 (master): [ruby/psych] Introduce `Psych.unsafe_load`
https://git.ruby-lang.org/ruby.git/commit/?id=c7c2ad5749 From c7c2ad5749f7f0767ef38be160f4b391228396c1 Mon Sep 17 00:00:00 2001 From: Aaron Patterson <tenderlove@r...> Date: Mon, 10 May 2021 09:50:06 -0700 Subject: [ruby/psych] Introduce `Psych.unsafe_load` 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. https://github.com/ruby/psych/commit/cb50aa8d3f --- ext/psych/lib/psych.rb | 8 +++--- ext/psych/lib/psych/versions.rb | 4 +-- test/psych/helper.rb | 30 +++++++++++++++------- test/psych/test_alias_and_anchor.rb | 12 ++++----- test/psych/test_array.rb | 6 ++--- test/psych/test_coder.rb | 14 +++++----- test/psych/test_date_time.rb | 4 +-- test/psych/test_deprecated.rb | 4 +-- test/psych/test_exception.rb | 8 +++--- test/psych/test_hash.rb | 16 ++++++------ test/psych/test_marshalable.rb | 6 ++--- test/psych/test_merge_keys.rb | 22 ++++++++-------- test/psych/test_object.rb | 4 +-- test/psych/test_object_references.rb | 12 ++++++--- test/psych/test_omap.rb | 4 +-- test/psych/test_psych.rb | 22 ++++++++-------- test/psych/test_ractor.rb | 2 +- test/psych/test_serialize_subclasses.rb | 4 +-- test/psych/test_set.rb | 6 ++--- test/psych/test_string.rb | 14 +++++----- test/psych/test_struct.rb | 6 ++--- test/psych/test_yaml.rb | 12 ++++----- test/psych/test_yaml_special_cases.rb | 2 +- test/psych/test_yamlstore.rb | 45 ++++++++++++++++++++++----------- test/psych/visitors/test_to_ruby.rb | 4 +-- test/psych/visitors/test_yaml_tree.rb | 8 +++--- 26 files changed, 156 insertions(+), 123 deletions(-) diff --git a/ext/psych/lib/psych.rb b/ext/psych/lib/psych.rb index cedf0a4..34d2218 100644 --- a/ext/psych/lib/psych.rb +++ b/ext/psych/lib/psych.rb @@ -271,7 +271,7 @@ module Psych https://github.com/ruby/ruby/blob/trunk/ext/psych/lib/psych.rb#L271 # 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 @@ -281,6 +281,7 @@ module Psych https://github.com/ruby/ruby/blob/trunk/ext/psych/lib/psych.rb#L281 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 @@ -577,11 +578,12 @@ module Psych https://github.com/ruby/ruby/blob/trunk/ext/psych/lib/psych.rb#L578 # 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 diff --git a/ext/psych/lib/psych/versions.rb b/ext/psych/lib/psych/versions.rb index b0ec018..acb2133 100644 --- a/ext/psych/lib/psych/versions.rb +++ b/ext/psych/lib/psych/versions.rb @@ -1,8 +1,8 @@ https://github.com/ruby/ruby/blob/trunk/ext/psych/lib/psych/versions.rb#L1 - # 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 diff --git a/test/psych/helper.rb b/test/psych/helper.rb index b118b31..0643139 100644 --- a/test/psych/helper.rb +++ b/test/psych/helper.rb @@ -41,24 +41,30 @@ module Psych https://github.com/ruby/ruby/blob/trunk/test/psych/helper.rb#L41 # 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 ) @@ -69,9 +75,15 @@ module Psych https://github.com/ruby/ruby/blob/trunk/test/psych/helper.rb#L75 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 diff --git a/test/psych/test_alias_and_anchor.rb b/test/psych/test_alias_and_anchor.rb index 91c09df..81ebd66 100644 --- a/test/psych/test_alias_and_anchor.rb +++ b/test/psych/test_alias_and_anchor.rb @@ -19,7 +19,7 @@ module Psych https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L19 - *id001 - *id001 EOYAML - result = Psych.load yaml + result = Psych.unsafe_load yaml result.each {|el| assert_same(result[0], el) } end @@ -33,7 +33,7 @@ EOYAML https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L33 - *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) @@ -50,7 +50,7 @@ EOYAML https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L50 - *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) @@ -62,7 +62,7 @@ EOYAML https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L62 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 @@ -73,7 +73,7 @@ EOYAML https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L73 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) @@ -87,7 +87,7 @@ EOYAML https://github.com/ruby/ruby/blob/trunk/test/psych/test_alias_and_anchor.rb#L87 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) diff --git a/test/psych/test_array.rb b/test/psych/test_array.rb index f2bbdca..28b76da 100644 --- a/test/psych/test_array.rb +++ b/test/psych/test_array.rb @@ -24,7 +24,7 @@ module Psych https://github.com/ruby/ruby/blob/trunk/test/psych/test_array.rb#L24 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 @@ -42,13 +42,13 @@ module Psych https://github.com/ruby/ruby/blob/trunk/test/psych/test_array.rb#L42 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 (... truncated) -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/