Anatomy of an Exploit: An In-depth Look at the Rails YAML Vulnerability | Hacker News Reader

Anatomy of an Exploit: An In-depth Look at the Rails YAML Vulnerability

rubysource.com

50 pointsthinkbohemian24 comments

Ok, at risk of turning this post into a flame war, I feel I'm obligated to call out what it is:

    Ruby devs: stop making excuses!

Security is hard, but there are also a lot of very simple and effective ways to minimize vulnerabilities, e.g. a mere excising of your cerebral cortex for a few minutes before you make a decision is a very good way in preventing security vulnerabilities from being introduced in the first place.

The mere fact of deciding to default to using YAML as the default JSON parser without disabling arbitrary object construction is a mind-numbingly dumb decision. What's more fucked up is the fact that this commit actually made it into the code base, and it's not even a recent decision. Apparently it's been in the code base as far back as the 2.3 days. Basically, thousands and thousands of Rails devs have looked and used this "feature" and believed it's not a bug. This is a systematic and cultural failure and crime against reason perpetrated by the Rails community, which also happens to be a vast majority of the Ruby community. It's time you guys learn some proper security 101.

I think you're being downvoted because you set up a bit of a strawman there. By my read, the central focus of this article is to explain a very dangerous vulnerability in an accessible way. It didn't come across as having made a lot of excuses.

I'm just going to quote directly from the post:

    The holes in Rails XML and JSON parsers for different
    vulnerable versions have been fixed, and some have asked 
    why they weren’t detected and patched earlier. The simple 
    answer is: security is hard. These issues are only 
    obvious in retrospect. Rails and Ruby aren’t any less 
    secure than other frameworks and languages. Security 
    vulnerabilities are bugs at their core, and very 
    difficult to detect. There is almost guaranteed to be 
    insecure software on your laptop/phone/server/garage-
    door-opener somewhere – it just hasn’t been discovered 
    yet.

I don't disagree with the last 2 sentences. But I don't agree with the first part of this paragraph at all and I have to call it out. I don't agree my example was a strawman, Rails has a track record of introducing 8 code execution vuln and 8 SQL injection vuln. Django has 1.

The fact is, it's not just this article that was making excuses, it's the Rails devs that have their bottom-line at stake that have come out of the woodwork since Jan that have pissed me off. Example: This guy who wrote a response to the "What The Rails Security Issue Means For Your Startup" post:

    http://isotope11.com/blog/what-the-rails-security-issue-shouldnt-mean-for-your-startup

They are also prevalent in the past HN threads discussing Rails vulns.

Note to Rails devs: if you want to secure your bottom-line, you should probably do your part in writing secure(r) software.

There's very little you're saying here that I disagree with. I also agree with you on the matter of Ruby on Rails being less secure than other frameworks. A friend of mine quipped the other day that it's time for Ruby devs to "take of their brogramming cool-shades and embrace some of what the neckbeards have been telling us."

I just don't interpret "security is hard" as an excuse. You just repeated the phrase yourself, yet your central point seems to hinge on that being an excuse. You saw fit to put it in its own block and indent it.

There are also a community of Ruby developers who are trying very hard to improve the situation. I'm sure there are more, but the group at the forefront of my mind is Ronin Ruby [1].

In order to improve, we must first admit that we have failed. For my part in the Ruby community, I humbly admit that we can do better.

[1]:http://ronin-ruby.github.com

The central point I'm trying to make is this article is being disingenuous. Security is hard, but not so hard that a minute or two of thinking and filing a PR won't prevent the YAML exploits. I don't actually think the Rails community is dumb, that's why this whole thing pissed me off so much because the fact that it was even introduced and existed for so long tells me that thousands of Rails devs have just decided to turn a blind eye over the years.

I'm well aware the Ronin guys BTW, I have mentioned them in a comment in another HN thread a few days back and called for help for those 2 guys.

bauland42 who is behind http://www.rorsecurity.info/ also deserves an honorable mention.

Good luck Rubyists.

One of the best lines is: "Security vulnerabilities are bugs at [frameworks' and languages'] core, and very difficult to detect." It's a good illustration of why "security" can almost never be a "feature" added on top of software. I try to help my teams understand that security is an emergent property of the system. Thus, if a system is to be secure, it must be accounted for throughout the design and development of a product.

I once saw an application that used Java's standard PRNG to create session IDs used for authentication after a user logged in. They tried to fix the fundamental security flaw by appending a time stamp, and then hashing that value. The powers at the time didn't believe it was a vulnerability until I did some white hat hacking. I wrote a simple app that retrieved a couple of session IDs and calculated the seed of the random number generator from them. It was a simple step then to guess session IDs of subsequent logins and impersonate those users. They quickly moved to replace the whole design with a fundamentally more secure solution.

> This is desired functionality by the creators of YAML, since it gives developers the ability to write and read Ruby objects to disk, like an object database.

It is? I don't think the creators of YAML exactly had Ruby in mind.

This isn't a problem with YAML, it is a problem with how the Ruby/Rails stack handles YAML.

"In contrast, YAML’s foremost design goals are human readability and support for serializing arbitrary native data structures."

- YAML Spec: http://yaml.org/spec/1.2/spec.html#id2763035

It isn't an issue with Ruby's implementation, but rather the spec itself.

You're splitting hairs here, and ignoring the rest of the article. YAML isn't evil or a "problem" per say. It's only when you pair YAML with un-sanitized user input that we get into problems.

Where here "sanitize" means "disable most of the features of the YAML parser".

The spec seems safe for data models where instantiating a data structure does not mean running arbitrary code (ie, constructors). For example, ML or Haskell abstract data types, or 'bless' in Perl.

I'm not sure what you mean by "data models where instantiating a data structure does not mean running arbitrary code". I can't think of a basic data type (a hard thing to define, btw) in Ruby that doesn't have a constructor, but I'm not sure that's what matters. Any attribute assignment in Ruby can be made dangerous if written in a way that wraps dangerous code in a attribute assignment methods.

What I think has gotten Rubyists in trouble is that we forgot YAML really is just serialization. Here's some advice that seems obvious now, but a few weeks ago would have inspired a "wha?" or a "huh?" from a lot of developers.

Don't pass anything to YAML::load that you wouldn't pass to Marshal::load.

Consider for a moment how someone would react to passing anything from HTTP post to Marshal::load. It seems obvious. It is obvious.

YAML is serialization. Whether or not serialization can be made safe is for people much, much smarter than me to figure out.

> This behavior isn’t inherently unsafe, after all we had to manually build our exploit string and manually instantiate our class. The problem only comes when we put all of these things together.

This is very much the wrong way to think about security. Weakness will be exploited. An attacker only has to find one path past your defenses; you have to guard every possible door. Saying, "look how strange and unlikely the interaction of these weaknesses was," misses the point. Code interacts lots of ways, and if it hadn't been this interaction, it could have been another.

Treating user data as untrustworthy is a good start, and you should do that. But you also ought to make each piece of your code base secure and robust, even if you can't think of how it would be exploited, now.

That was my implied point of the whole article. An attacker can use your code against you in unexpected ways.

It's probably difficult to think of all of the ways in which your code base can be exploited from the get-go.

If you want the straight-to-the-point version, postmodern wrote up a bit more technical version here: http://ronin-ruby.github.com/blog/2013/01/28/new-rails-poc.h...

I'm a ruby noob, but if I read this right it means that when you deserialize a YAML string into an object it will also create methods from ruby code inside the YAML string?

Does a serialization format really need this functionality? it strikes me as something that is clearly dangerous and you would be better simply to deserialize data members?

Is it a common requirement to pass code around as strings?

You aren't exactly reading it right. The string is deserialized as a string, specifically as a hash key. However, there are a number of classes in Rails (and presumably other Ruby projects of similar complexity) which can be tricked into evaluating string properties as code when they are created, and ActionDispatch::Routing::RouteSet::NamedRouteCollection is one.

The general lesson taken is not that it is dangerous to ever use eval in any class ever, because that is really useful, but that it is dangerous to allow arbitrary instantiation of classes, because that's pretty obvious even without this exploit. For example, even without code execution you could use this same trick to DoS the server by instantiating a bunch of heavy classes which won't get garbage collected.

Meh, I kind of think both lessons are valid.

Writing code that requires 'eval' to do it's thing has given most of us a dirty feeling since way back in 1990s perl days. It was generally considered a 'code smell'.

I find it odd that several places in Rails use instance_eval(string) or equivalent, especially places that generally _could_ get by, with identical apis, but implementations that use other sorts of dynamic method-defining behavior, not eval(string). It is odd, and I think probably a bad decision.

But yes, the lesson of this stuff is certainly that allowing untrusted input to be de-serialized into arbitrary objects is always unacceptable, because you can't _count_ on there being no class in your load path that refrains from using eval in a way dangerous with that combo, as well as DoS issues as you note, etc. Yes, this is true, no matter what.

But at the same time... I think using eval(string) probably is _almost_ always a bad idea too, it's just asking for trouble. (never say never, but you better really have no other option).

> The general lesson taken is not that it is dangerous to ever use eval in any class ever, because that is really useful

Ah, then this is, at its root, a language design deficiency. If there's something useful that can only be accomplished conveniently by calling 'eval' in a library routine whose job is not specifically to evaluate code, then the language needs a better way to do whatever that is.

In the Lisp world we tell people: never, ever call 'eval', the sole exception being that you're writing an interpreter. It is sound advice. Unless you're intentionally writing an interpreter, there is a better way to accomplish whatever you're trying to do.

Ah, a downvote. Yeah, I thought this might not be a popular message.

I have many years of experience in Lisp, and what I'm telling you is the longstanding consensus among Lisp experts. You should listen to us. Ruby is not so different from Lisp that the lesson is inapplicable.

I stand by my comment.

> However, there are a number of classes in Rails (and presumably other Ruby projects of similar complexity) which can be tricked into evaluating string properties as code when they are created, and ActionDispatch::Routing::RouteSet::NamedRouteCollection is one.

And that is just insane. But I come from PHP where eval = evil

Why Rubyists don't care what they are evaling as long as what they personally intend to pass into eval is useful?

Rather than creating new methods for a class, YAML.load can be used to call one of a few specific methods ([]=, init_with, or yaml_initialize) for the specified class. This exploit found a class where string arguments to the []= method are inserted into an eval() block, thus becoming code.

A few more details here: http://blog.gemfury.com/post/42259456238/rubygems-vulnerabil...

Security and good software engineering principles are perhaps not the first focus in the Ruby community, and that is a shame. Yes it is nice to have cool frameworks with lots of magic, and we can create a blog in one command. But none of that matters when you end up in infinite patch cycle. Remember poor design choices mean technical debt for a long time.

def double(number) return number * 2 end

double(10) # => 20

double("foo") # => "foofoo"

It's called polymorphism. `*` is a method.

Learn how the language works before you write an article about it proclaiming your ignorance.

Reply on news.ycombinator.com