4 ms·

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 pr

by 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.
jrochkind1 14y ago
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).
scotty79 14y ago
> 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?

by Cushman 14y ago

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.

jrochkind1 14y ago

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

scotty79 14y ago

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