Use Psych.safe_load by default by tenderlove · Pull Request #487 · ruby/psych

tenderlove · 2021-05-10T16:52:34Z

Psych.load is not safe for use with untrusted data. Too many
applications make the mistake of using Psych.load with untrusted data
and that ends up with some kind of security vulnerability.

This commit changes the default Psych.load to use safe_load. Users
that want to parse trusted data can use Psych.unsafe_load.

Psych.load or YAML.load are susceptible to a Remote Code Execution (or RCE) flaw. The problem isn't with the load functions themselves, but they can be used to abuse other objects that are in the system to escalate to an RCE. This blog post goes in to details about how YAML can be used in conjunction with RubyGems to execute arbitrary code. If someone tries to load YAML from an untrusted source, bad actors could use this issue to execute arbitrary code in the target system.

Psych provides a method called "safe_load" which can't be used to launch such an attack. The proposal here is to change the default such that "load" is now "safe_load". Of course the drawback is that "safe_load" is much more limited than "load", and making this change could break existing code.

Also it's probably worth mentioning that the YAML load functions could be used with other objects in the system besides RubyGems to escalate to an RCE. Also worth mentioning that anything that can be used to load arbitrary objects (like Marshal.load) can be abused in the same way. So neither changing RubyGems nor changing to a different serialization scheme (like Marshal) would plug this hole.

tenderlove · 2021-05-11T16:00:35Z

For context, Psych.safe_load is more close to JSON loading. It doesn't support Symbols, Dates, Objects, references, etc. The only things it supports by default are:

TrueClass
FalseClass
NilClass
Numeric
String
Array
Hash

stevecrozz · 2021-05-11T16:37:51Z

I support this change, but what do you plan to do with the version number? Feels like a major change.

tenderlove · 2021-05-11T16:46:32Z

I support this change, but what do you plan to do with the version number? Feels like a major change.

Yes, definitely major version change.

SamSaffron · 2021-05-11T22:29:13Z

I am extremely supportive of switching to safe by default, even going through a full rename dance here.

deprecate safe_load in favor of load
introduce unsafe_load

lol ... I just read the PR looks like you are very close already, only change I recommend adding is deprecating safe_load

tenderlove · 2021-05-11T22:42:16Z

lol ... I just read the PR looks like you are very close already, only change I recommend adding is deprecating safe_load

I've made load slightly different from safe_load. load allows all Symbols where safe_load doesn't. I'm not sure if it's a good idea or not. It's still possible for some symbols to never be garbage collected, but they have to go through a C extension that makes them that way. However, I suspect many (most?) people use symbols in their YAML and this would trip them up. It seems like a very rare issue, so maybe allowing symbols is "safe enough"? cc @evanphx

SamSaffron · 2021-05-12T00:12:18Z

It seems like a very rare issue, so maybe allowing symbols is "safe enough"?

Yeah sounds like at worst this is allowing some cascade of components to cause memory bloat. Enormously better place to be than RCE friendly by default.

I guess people like this kind of stuff and tend to use it?

---
:a: 1

One concern is that the "load" then is less cross compatible with other frameworks. :a: 1 in golang/js/rust will be treated as the string :a. Maybe load(str, allow_symbols: true) and disable by default? Not sure. Would be a far bigger breaking change.

tenderlove · 2021-05-12T00:28:36Z

One concern is that the "load" then is less cross compatible with other frameworks. 🅰️ 1 in golang/js/rust will be treated as the string :a.

That's true, but it's already the current behavior. For example:

YAML.load(<<-eostr)
---
:a: 1
eostr

The above code will result in the same data structure before and after this PR is merged. Allowing symbols is a compromise for compatibility / non-surprising behavior (as it's what people expect today). Also, interlanguage compatibility hasn't really been a priority for the library.

I'm leaning towards allowing symbols for YAML.load (but not YAML.safe_load) as I think that will give the best "backwards compatibility plus safety" bang for our buck.

tenderlove · 2021-05-12T00:31:27Z

hashie/hashie#473

k0kubun · 2021-05-12T04:08:00Z

+1 for the interface change.

How about releasing unsafe_* variants alone first though? Being able to explicitly rewrite load to safe_load or unsafe_load (where unsafe_ is really necessary) before switching the behavior of load would be useful to minimize the impact of breaking changes by the upgrade.

tenderlove · 2021-05-12T15:04:51Z

How about releasing unsafe_* variants alone first though? Being able to explicitly rewrite load to safe_load or unsafe_load (where unsafe_ is really necessary) before switching the behavior of load would be useful to minimize the impact of breaking changes by the upgrade.

Yes, that is a great idea. I will do it!

Psych.load is not safe for use with untrusted data. Too many applications make the mistake of using `Psych.load` with untrusted data and that ends up with some kind of security vulnerability. This commit changes the default `Psych.load` to use `safe_load`. Users that want to parse trusted data can use Psych.unsafe_load.

tenderlove · 2021-05-13T18:18:51Z

I shipped this in v4.0.0. Thanks!

lzap · 2021-05-17T06:52:09Z

Friday, our CI is red. Only if there was a better week day to push big releases out, hmmm 🤔

Well, a mistake in our Gemfile, we will fix it, and the code. Thank you for taking care of safety of all of us! Cheese, I mean, cheers! :-)

Follow ruby/psych#487. This PR fixes the following Ruby 3.1.0-dev build matrix error. ```console % ruby -ryaml -ve 'p Psych::VERSION' ruby 3.1.0dev (2021-05-17T10:51:51Z master ee611341c9) [x86_64-darwin19] "4.0.0" % bundle exec rspec ./spec/rubocop/cli/options_spec.rb (snip) Failures: 1) RuboCop::CLI options --show-cops with no args prints the current configuration Failure/Error: printed_config = YAML.load(out.join) # rubocop:disable Security/YAMLLoad Psych::DisallowedClass: Tried to load unspecified class: Regexp Shared Example Group: "prints config" called from ./spec/rubocop/cli/options_spec.rb:1104 # (eval):2:in `regexp' # ./spec/rubocop/cli/options_spec.rb:1027:in `block (4 levels) in <top (required)>' # ./spec/support/cli_spec_behavior.rb:26:in `block (2 levels) in <top (required)>' # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (4 levels) in <top (required)>' # ./lib/rubocop/rspec/shared_contexts.rb:30:in `chdir' # ./lib/rubocop/rspec/shared_contexts.rb:30:in `block (3 levels) in <top (required)>' # ./lib/rubocop/rspec/shared_contexts.rb:7:in `block (2 levels) in <top (required)>' Finished in 25.97 seconds (files took 1.02 seconds to load) 117 examples, 1 failure Failed examples: rspec ./spec/rubocop/cli/options_spec.rb[1:13:1:4] # RuboCop::CLI options --show-cops with no args prints the current configuration ``` https://app.circleci.com/pipelines/github/rubocop/rubocop/4637/workflows/d6722067-28c1-4740-a1b8-abea06edab0d/jobs/185620

Psych 4 load in safe mode by default: ruby/psych#487 So aliases need to be explicitly allowed

Starting In Psych 4.0.0 YAML.load behaves like YAML.safe_load. To preserve compatibility Sinatra::ConfigFile now uses YAML.unsafe_load if available. ruby/psych#487

This PR fixes a false positive for `Security/YamlLoad` when using Ruby 3.1+ (Psych 4). And it also makes the examples a bit more specific. Ruby 3.1+ (Psych 4) uses `Psych.load` as `Psych.safe_load` by default. ruby/psych#487 We may consider removing this cop in the future, but not yet.

Ruby 3.1 contains Psych version 4 as the default YAML parsing library. This has a backwards compatibility break where YAML.load_file behaviour has changed from an unsafe load to a safe load [1]. This causes an exception to occur when using GOV.UK Puppet Hieradata YAML files are parsed, as these make use of aliases - which is a feature that needs to be enabled for YAML in safe load. To resolve this issue I've set a conditional to check if the safe_load method exists on the YAML object. I did consider whether it could be resoled by pinning the psych dependency, but this seemed a bit delicate as this tool has been authored to have no external dependencies and is installed explicitly without them [2]. I couldn't identify an approach to apply a test for this that was consistent with this repo's code and test conventions. Due to the fluctuating standard library it'd have been a little contrived. Before: ``` ➜ ~ govuk-connect app-console -e integration asset-manager Connecting to the app console for asset-manager, in the integration environment The relevant hosting provider is aws /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:430:in `visit_Psych_Nodes_Alias': Unknown alias: node_class (Psych::BadAlias) from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:30:in `visit' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:6:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:35:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:345:in `block in revive_hash' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `each' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `each_slice' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `revive_hash' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:167:in `visit_Psych_Nodes_Mapping' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:30:in `visit' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:6:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:35:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:345:in `block in revive_hash' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `each' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `each_slice' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:343:in `revive_hash' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:167:in `visit_Psych_Nodes_Mapping' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:30:in `visit' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:6:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:35:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:318:in `visit_Psych_Nodes_Document' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:30:in `visit' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/visitor.rb:6:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych/visitors/to_ruby.rb:35:in `accept' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych.rb:335:in `safe_load' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych.rb:370:in `load' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych.rb:671:in `block in load_file' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych.rb:670:in `open' from /usr/local/Cellar/ruby/3.1.0/lib/ruby/3.1.0/psych.rb:670:in `load_file' ``` After: ``` ➜ ~ govuk-connect app-console -e integration asset-manager Connecting to the app console for asset-manager, in the integration environment The relevant hosting provider is aws The relevant node class is backend ``` [1]: ruby/psych#487 [2]: https://github.com/alphagov/homebrew-gds/blob/de1fb95fbda6219ad7158ab0c71253dcb55822d9/Formula/govuk-connect.rb#L12

tenderlove requested a review from hsbt May 10, 2021 16:52

tenderlove force-pushed the default-unsafe branch from f3cc8b1 to f351e51 Compare May 11, 2021 20:33

k0kubun reviewed May 12, 2021

View reviewed changes

Comment thread lib/psych.rb Outdated

tenderlove added 3 commits May 13, 2021 10:52

Bump version

1df86a2

remove deprecated interface

0767227

tenderlove force-pushed the default-unsafe branch from 818f830 to 0767227 Compare May 13, 2021 17:58

tenderlove merged commit 216f94f into master May 13, 2021

tenderlove deleted the default-unsafe branch May 13, 2021 18:09

tenderlove mentioned this pull request May 14, 2021

Release of 4.0.0 breaks yaml files with dates #489

Closed

KonnorRogers mentioned this pull request May 15, 2021

Remove safe_yaml gem in favour of using Psych which is in the stdlib bridgetownrb/bridgetown#303

Merged

lzap mentioned this pull request May 17, 2021

Fixes #27604 - added Fedora|Redhat CoreOS and IM theforeman/foreman#8042

Merged

koic mentioned this pull request May 17, 2021

Fix a build error for Ruby 3.1.0-dev rubocop/rubocop#9806

Merged

8 tasks

byroot added a commit to rails/webpacker that referenced this pull request May 18, 2021

Fix compatibility with Psych 4

671c6f1

Psych 4 load in safe mode by default: ruby/psych#487 So aliases need to be explicitly allowed

byroot mentioned this pull request May 18, 2021

Fix compatibility with Psych 4 rails/webpacker#3024

Merged

byroot added a commit to rails/webpacker that referenced this pull request May 18, 2021

Fix compatibility with Psych 4

f8dbc91

Psych 4 load in safe mode by default: ruby/psych#487 So aliases need to be explicitly allowed

onk added a commit to onk/sinatra that referenced this pull request Jan 29, 2022

Fix compatibility with Psych >= 4

e1fb977

Starting In Psych 4.0.0 YAML.load behaves like YAML.safe_load. To preserve compatibility Sinatra::ConfigFile now uses YAML.unsafe_load if available. ruby/psych#487

onk mentioned this pull request Jan 29, 2022

Fix compatibility with Psych >= 4 sinatra/sinatra#1740

Closed

koic mentioned this pull request Feb 17, 2022

Fix a false positive for Security/YamlLoad rubocop/rubocop#10424

Merged

8 tasks

kevindew mentioned this pull request Feb 22, 2022

Provide compatibility with Ruby 3.1 alphagov/govuk-connect#69

Merged

odaki mentioned this pull request Mar 4, 2022

pandoc2reviewタスクがエラーになる TechBooster/ReVIEW-Template#84

Closed

yui-har mentioned this pull request Mar 4, 2022

Psych 4: Psych::BadAlias exception when unserializing a config/s3.yml redmica/redmica_s3#32

Merged

tomgi mentioned this pull request Mar 9, 2022

This gem doesn't work on Ruby 3.1 due to a breaking change of YAML#load krisleech/wisper-sidekiq#34

Closed

gyugyu mentioned this pull request Mar 16, 2022

use YAML.unsafe_load to support Ruby 3.1/Psych 4 middleman/middleman#2541

Closed

chaaaaarlotte mentioned this pull request Apr 17, 2022

Support Psych4.x itamae-kitchen/itamae#347

Merged

jordansissel mentioned this pull request May 1, 2022

psych/class_loader.rb:99:in `find': Tried to load unspecified class: Gem::Specification (Psych::DisallowedClass) jordansissel/fpm#1895

Closed

sixtyfive mentioned this pull request May 24, 2022

Doesn't work after upgrading to Ruby 3.1 carsomyr/rbenv-bundler#57

Open

NickWalt01 mentioned this pull request May 24, 2022

update tech-docs-github-pages-publisher to 1.5 ministryofjustice/technical-guidance#150

Merged

jdelStrother mentioned this pull request Jun 20, 2022

Einhorn incompatible with ruby 3.1 / psych 4 sidekiq/sidekiq#5400

Closed

noniq mentioned this pull request Jul 3, 2022

Date error when running latest middleman-blog against middleman 4 alpha 6 middleman/middleman-blog#238

Closed

alpaca-tc mentioned this pull request Jul 13, 2022

Support Psych v4.0.0 laserlemon/figaro#288

Open

kotsiuruba mentioned this pull request Jul 21, 2022

handle aliases on YAML load SouthernMade/sm_app_config#2

Merged

tnir mentioned this pull request Aug 1, 2022

Workaround for psych 4.0 in tests n-ronn/nronn#15

Merged

stanhu mentioned this pull request Mar 13, 2023

Fix deprecated interface for YAML.safe_load gettalong/kramdown#781

Closed

krauselukas mentioned this pull request Apr 27, 2023

Fix loading of yaml content for ruby version newer than 3.1.0 tzmfreedom/json_refs#10

Merged

DanielGilchrist mentioned this pull request Jul 31, 2023

Ruby 3.1+ / Psych 4+ compatibility TandaHQ/migration_signature#1

Merged

gretchenziegler mentioned this pull request Oct 11, 2023

chore(PUI-20700): Update YAML loading for Ruby 3 compatibility mdx-dev/sunspot#25

Merged

Ifraah mentioned this pull request Feb 7, 2025

Psych 4.0 - Allow aliases for YAML load bodyshopbidsdotcom/ssm_config#23

Merged

rosa mentioned this pull request May 9, 2025

Anchors do not work in recurring jobs yaml file rails/solid_queue#556

Closed

p8 mentioned this pull request Nov 24, 2025

Add Rack::QueryParser::Params to YAML.load permitted_classes in specs rack-app/rack-app#58

Merged

Navigation Menu

Uh oh!

Use Psych.safe_load by default - #487

Use Psych.safe_load by default#487
tenderlove merged 3 commits into
masterfrom
default-unsafe

tenderlove commented May 10, 2021 •

edited

Loading

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

stevecrozz commented May 11, 2021

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

SamSaffron commented May 11, 2021 •

edited

Loading

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

SamSaffron commented May 12, 2021

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

k0kubun commented May 12, 2021

Uh oh!

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

tenderlove commented May 13, 2021

Uh oh!

lzap commented May 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Uh oh!

Conversation

tenderlove commented May 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

stevecrozz commented May 11, 2021

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

SamSaffron commented May 11, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tenderlove commented May 11, 2021

Uh oh!

SamSaffron commented May 12, 2021

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

k0kubun commented May 12, 2021

Uh oh!

Uh oh!

tenderlove commented May 12, 2021

Uh oh!

tenderlove commented May 13, 2021

Uh oh!

lzap commented May 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

tenderlove commented May 10, 2021 •

edited

Loading

SamSaffron commented May 11, 2021 •

edited

Loading