7 ms·

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

50 points by thinkbohemian 14y ago

DHowett 14y ago
> 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.
- thinkbohemian 14y ago
  "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 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.
  - tptacek 14y ago
    Where here "sanitize" means "disable most of the features of the YAML parser".
  - aphexairlines 14y ago
    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.
    - bradleyland 14y ago
      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.
jiggy2011 14y ago
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?
- Cushman 14y ago
  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.
  - ScottBurson 14y ago
    > 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.
    - ScottBurson 14y ago
      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.
zopa 14y ago
> 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.
- thinkbohemian 14y ago
  That was my implied point of the whole article. An attacker can use your code against you in unexpected ways.
- jiggy2011 14y ago
  It's probably difficult to think of all of the ways in which your code base can be exploited from the get-go.
dguido 14y ago
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.html http://ronin-ruby.github.com/blog/2013/01/28/new-rails-poc.h...
hawleyal 14y ago
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.
wyuenho 14y ago
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.
- bradleyland 14y ago
  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.
  - wyuenho 14y ago
    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.
derricki 14y ago
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.
deleted 14y ago
[deleted]
static_typed 14y ago
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.

50 points by thinkbohemian 14y ago

> 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.

thinkbohemian 14y ago
"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 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.
- tptacek 14y ago
  Where here "sanitize" means "disable most of the features of the YAML parser".
- aphexairlines 14y ago
  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.
  - bradleyland 14y ago
    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.

jiggy2011 14y ago

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?

Cushman 14y ago
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.
- ScottBurson 14y ago
  > 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.
  - ScottBurson 14y ago
    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.

zopa 14y ago

> 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.

thinkbohemian 14y ago
That was my implied point of the whole article. An attacker can use your code against you in unexpected ways.
jiggy2011 14y ago
It's probably difficult to think of all of the ways in which your code base can be exploited from the get-go.

dguido 14y ago

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.html http://ronin-ruby.github.com/blog/2013/01/28/new-rails-poc.h...

hawleyal 14y ago

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.

wyuenho 14y ago

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.

bradleyland 14y ago
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.
- wyuenho 14y ago
  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.

derricki 14y ago

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.

deleted 14y ago

[deleted]

static_typed 14y ago

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.