Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use YAML.safe_load #1680

Merged
merged 1 commit into from
Feb 27, 2015
Merged

Use YAML.safe_load #1680

merged 1 commit into from
Feb 27, 2015

Conversation

croaky
Copy link

@croaky croaky commented Feb 26, 2015

There's a vulnerability in YAML.load
which can enable arbitrary code to be run.
YAML.safe_load does not deserialize unsafe classes.

Related reading:

While Rubocop is generally run locally,
it could be used in server-side, hosted environments.

I can't think of a downside to using the safe_load version.

This change is intended to maintain backwards compatibility
with Ruby versions that appear to be supported by Rubocop
(judging by the .travis.yml file).

There's a vulnerability in `YAML.load`
which can enable arbitrary code to be run.
`YAML.safe_load` does not deserialize unsafe classes.

Related reading:

* http://blog.codeclimate.com/blog/2013/01/10/rails-remote-code-execution-vulnerability-explained/
* ruby/psych#119
* http://docs.ruby-lang.org/en/2.1.0/Psych.html#method-c-safe_load

While Rubocop is generally run locally,
it could be used in server-side, hosted environments.

I can't think of a downside to using the `safe_load` version.

This change is intended to maintain backwards compatibility
with Ruby versions that appear to be supported by Rubocop
(judging by the `.travis.yml` file).

`Rexexp` needs to be whitelisted.
@bquorning
Copy link
Contributor

You probably need to add Regexp to the whitelist_classes argument. A Ruby regex is used for the Style/WordArray cop: https://github.com/bbatsov/rubocop/blob/master/config/default.yml#L582-L585

@croaky
Copy link
Author

croaky commented Feb 26, 2015

@bquorning Yup, sorry. Just realized that. Green locally. Pushed.

bbatsov added a commit that referenced this pull request Feb 27, 2015
@bbatsov bbatsov merged commit 2dff15e into rubocop:master Feb 27, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 27, 2015

👍

@neilang
Copy link

neilang commented Jun 16, 2015

Looking at the links to the documentation, Regexp is not a supported deserializable type.

I'm using ruby 2.2.2 which is producing an error because it looks like YAML.safe_load converts WordRegex to a string. Specifically the error is type mismatch: String given when it hits this line.

@jonas054
Copy link
Collaborator

@neilang I don't get the same problem with my installation 0.32.0 (using Parser 2.2.2.5, running on ruby 2.2.2 x86_64-linux). What do you get if you run

p YAML.safe_load("!ruby/regexp '/\A[\p{Word}]+\z/'", [Regexp])

in a program or irb or something?

My understanding is that Regexp deserialization is not supported by default, which is why the [Regexp] argument to YAML.safe_load is needed.

@neilang
Copy link

neilang commented Jun 22, 2015

@jonas054 I have a similar setup 0.32.0 (using Parser 2.2.2.5, running on ruby 2.2.2 x86_64-darwin14).

Digging deeper into this I've found that yaml is being patched by another gem in my bundle, which is the real cause of the issue.

require 'yaml'
p YAML.safe_load("!ruby/regexp '/\A[\p{Word}]+\z/'", [Regexp])
=> /A[p{Word}]+z/

require 'safe_yaml'
p YAML.safe_load("!ruby/regexp '/\A[\p{Word}]+\z/'", [Regexp])
=> "/A[p{Word}]+z/"

Sorry about that. Thanks for your help!

@robbyoconnor
Copy link

This is causing problems now for some reason... See #2055 -- I don't know enough to fix it...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants