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

YAML response middleware uses YAML.load #92

Closed
mattly opened this issue Jun 24, 2014 · 1 comment
Closed

YAML response middleware uses YAML.load #92

mattly opened this issue Jun 24, 2014 · 1 comment

Comments

@mattly
Copy link

mattly commented Jun 24, 2014

Was troubleshooting a problem with JSON handling, and looking through things, and came across this: https://github.com/lostisland/faraday_middleware/blob/master/lib/faraday_middleware/response/parse_yaml.rb

which IIRC is pretty unsafe. You probably want Psych.safe_load

@mislav mislav closed this as completed in 21046ad Oct 10, 2014
@mislav
Copy link
Contributor

mislav commented Oct 10, 2014

Thanks for the suggestions. I've added some docs but didn't change the default since that would mess with backwards compatibility. I know that opting into safety sucks, but people are generally using such middleware when talking to APIs over HTTPS from services they trust.

mislav added a commit that referenced this issue Jul 7, 2015
We can't just switch to `YAML.safe_load()` ourselves since that would
break backwards compatibility. For instance, `safe_load` returns nil for
empty yaml documents where `load` returns `false`. Also, `safe_load`
will refuse to parse Symbol keys since DoS attacks targeting symbols are
a real thread. Finally, not every Ruby version has a Psych that supports
`safe_laod`.

ruby/psych#119 (comment)

Fixes #92
petems pushed a commit to petems/faraday_middleware_safeyaml that referenced this issue Jul 3, 2017
An issue from June 2014,
lostisland#92, raises
the risks of the current `FaradayMiddleware::ParseYaml` middleware
which uses `YAML.load`. This method is very unsafe and exposes you
to remote code execution - see
ruby/psych#119 for discussion.

At the time, @mislav decided not to make this change to avoid
messing with backwards compatability.

I would suggest that we should revisit this decision - the risks
of this are very high, very few people are using this middleware
most likely, and it doesn't seem unreasonable to break this
as long as we are clear on the change in the changelog.

This does that by installing the `safe_yaml` gem, which is
compatible with all Ruby versions we support.
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

No branches or pull requests

2 participants