[ruby-core:103880] [Ruby master Bug#17866] Incompatible changes with Psych 4.0.0

Issue #17866 has been updated by tenderlovemaking (Aaron Patterson).


Eregon (Benoit Daloze) wrote in #note-4:
> byroot (Jean Boussier) wrote in #note-3:
> > It kinda is, as aliases allow for circular references which can cause some programs to end up in infinite loop.
> 
> Doesn't the YAML implementation handle/avoid such recursive/circular references?

No, you can definitely make recursive data structures:

```ruby
irb(main):006:0> x = []
=> []
irb(main):007:0> x << x
=> [[...]]
irb(main):008:0> puts Psych.dump x
--- &1
- *1
=> nil
irb(main):009:0> y = Psych.load Psych.dump x
=> [[...]] 
```

> So, unless you know of a YAML that crashes Psych.safe_load(..., aliases: true), I assume it's safe.

No circular aliases will cause `Psych.safe_load` to crash.  The problem is any code that *processes the return value* of `Psych.load`.  Sometime like this:

```ruby
require "psych"

def process thing
  thing.each do |item|
    case item
    when Array
      process item
    when Hash
      # ...
    when String
      # ...
    end
  end
end

user_input = DATA.read

process Psych.load(user_input)

__END__
--- &1
- *1

```

> if load would allow aliases by default it would make things much easier for everyone.

Ya, I agree.  If we can allow aliases but not recursive aliases, I think that would be great.  I'm not 100% sure how to do it though.

I'm ok with enabling aliases by default.  DoS attacks using recursive structures have only been theoretical. However, I'm very tired of dealing with YAML related security issues, so I'm not 100% sure. 


----------------------------------------
Bug #17866: Incompatible changes with Psych 4.0.0
https://bugs.ruby-lang.org/issues/17866#change-92008

* Author: hsbt (Hiroshi SHIBATA)
* Status: Assigned
* Priority: Normal
* Assignee: tenderlovemaking (Aaron Patterson)
* Backport: 2.6: UNKNOWN, 2.7: UNKNOWN, 3.0: UNKNOWN
----------------------------------------
Psych-4.0.0 changes `Psych.safe_load` by the default.

https://github.com/ruby/psych/pull/487

It breaks the several code like:

* https://github.com/ruby/ruby/commit/da5b28396397ace84d914cb188055cbeb46b8725
* https://github.com/ruby/ruby/commit/8e91b969df08b7a2eb27a5d6d38733eea42dc7ad
* https://github.com/ruby/ruby/commit/d8fd92f62024d85271a3f1125bc6928409f912e1
* https://github.com/ruby/ruby/commit/dfecc650c3f9bbd8b4fb0eefc1e3da65f151d3a8
* etc...

I and @mame investigate them. We found 2 issues.

1. `Symbol` is still ignored `Pysch.load`. It break many of code like configuration store. https://github.com/ruby/psych/blob/master/lib/psych.rb#L368 passes `Symbol` used by `permitted_classes`. But It's not working now. see https://github.com/ruby/psych/issues/490
2. `Pysch.load` restrict `Gem::Specification` or `RDoc::Options` by the default. Should we add them with`permitted_classes` to `Psych.load` or `Psych.load_file`? I'm not sure the right way about them.

@tenderlovemaking Do you have any ideas about the above concerns?




-- 
https://bugs.ruby-lang.org/

Unsubscribe: <mailto:[e-mail]?subject=unsubscribe>
<http://lists.ruby-lang.org/cgi-bin/mailman/options/ruby-core>