Feature #7677: YAML load mode that does instantiate Ruby

• Status: Closed
• Assignee: tenderlovemaking (Aaron Patterson)
• Target version: 2.6

[trans (Thomas Sawyer)]

See https://makandracards.com/makandra/892-never-use-yaml-load-with-user-input

I suggest that YAML.load and YAML.load_file have an optional mode that will allow the YAML to load but not instantiate !ruby/object: tags, nor any registered tags. To go with this there could be a way to see what the tag is after having been loaded.

On Wed, Jan 09, 2013 at 11:40:04AM +0900, trans (Thomas Sawyer) wrote:

Issue #7677 has been reported by trans (Thomas Sawyer).

---

Feature #7677: YAML load mode that does instantiate Ruby
https://bugs.ruby-lang.org/issues/7677

Author: trans (Thomas Sawyer)
Status: Open
Priority: Normal
Assignee:
Category: lib
Target version: next minor

See https://makandracards.com/makandra/892-never-use-yaml-load-with-user-input

I suggest that YAML.load and YAML.load_file have an optional mode that will allow the YAML to load but not instantiate !ruby/object: tags, nor any registered tags. To go with this there could be a way to see what the tag is after having been loaded.

Use Psych.parse, then you can inspect the AST.

--
Aaron Patterson
http://tenderlovemaking.com/

[trans (Thomas Sawyer)]

=begin
Is that a viable option for general usage?

Let me give an example of where this issue becomes a problem. I received an email a couple of days ago:

You may have read about the recent Rails security issue. I had no idea
YAML.load enabled remote code execution when given user input.

The same problem is in Gollum as a result of your page metadata pull
request that I approved. I had to disable it in Gollum today and
released 2.4.11 with the fix. Do you think it's worth updating page
metadata or should it be removed?

The conclusion of our conversation was pretty simple. YAML would have to go unless there is a fix, and JSON would be used instead. I hate to see that happen, but there isn't much I can do about it other then ask for a fix.

Some links related to this:

• http://www.insinuator.net/2013/01/rails-yaml/
• http://news.ycombinator.com/item?id=5028218
• https://github.com/github/gollum/pull/419

=end

On Fri, Jan 11, 2013 at 12:05:36AM +0900, trans (Thomas Sawyer) wrote:

Issue #7677 has been updated by trans (Thomas Sawyer).

=begin
Is that a viable option for general usage?

Let me give an example of there where this issue becomes a problem. I received an email a couple of days ago:

You may have read about the recent Rails security issue. I had no idea
YAML.load enabled remote code execution when given user input.

YAML.load does not enable remote code execution. You must use it in
conjunction with some other object that does something dangerous with
it. In the case of Rails, that would be module_eval:

https://github.com/rails/rails/blob/master/actionpack/lib/action_dispatch/routing/route_set.rb#L188-200

Any serialization scheme that will allow custom objects could be
impacted in the same way. It has to be serialization scheme PLUS some
dangerous operation.

The same problem is in Gollum as a result of your page metadata pull
request that I approved. I had to disable it in Gollum today and
released 2.4.11 with the fix. Do you think it's worth updating page
metadata or should it be removed?

The conclusion of our conversation was pretty simple. YAML would have to go unless there is a fix, and JSON would be used instead. I hate to see that happen, but there isn't much I can do about it other then ask for a fix.

If you'd like to help define what "safe yaml" means, there's a ticket
here:

https://github.com/tenderlove/psych/issues/119

--
Aaron Patterson
http://tenderlovemaking.com/

[trans (Thomas Sawyer)]

I added my concept of it to the issue (https://github.com/tenderlove/psych/issues/119).

Thanks.

By the way, the title of this issue should say "does NOT instantiate". Sorry.

[ko1 (Koichi Sasada)]

• Assignee set to tenderlovemaking (Aaron Patterson)

[tenderlovemaking (Aaron Patterson)]

• Status changed from Open to Closed
• % Done changed from 0 to 100

Psych.safe_load method has been introduced, which should deal with this issue. Thanks!