Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

Why the Latest Rails Exploit Is Indicative of a Bigger Problem

blog.sdelements.com

Post reply on HN
Print view View original

41–50 of 52 posts

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#41

Post by ScottBurson »

static_typed wrote: post #5

Serious developers take an interest in making things as simple as possible, ideally each piece have singular responsibilities, and no magic. Software engineering is hard, and it is heartening to see more people realise and promote awareness of some of the more dangerous anti-patterns we see in frameworks like Rails.

Let's get specific, because there's a point here that I think a lot of people are missing (including tptacek).

ActionDispatch::Routing::RouteSet::NamedRouteCollection.add calls 'eval' on the property name passed to it. This invites exploitation. It also fails 'static_typed's desideratum of each component having a single responsibility. And I don't think calling it "magic" is too strong.

I don't know to what extent the Ruby culture encourages calling 'eval' in random API routines like this, but when I previously argued here on HN that that had to be a bad idea, I got some pushback. I would certainly call this a dangerous anti-pattern.

EDITED to add: reference: http://rubysource.com/anatomy-of-an-exploit-an-in-depth-look...

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#42

Post by ScottBurson »

tptacek wrote: post #34
rambot wrote: post #27

Earlier quoted context omitted.

Where does the post suggest Rails is the only framework with bugs like this? The article opens with: "The latest Rails security flaw is example of a common anti-pattern. ... a similar issue may also exist in Python’s YAML parser ... I am reminded of similar flaws in other frameworks and libraries." I don't think the goal here was to pick on Rails, but to highlight that its recent security woes might be a sign of a bi…

I'm responding to the comment above, not the post.

I don't read the comment in question as picking on Rails either ("... frameworks like Rails", it says).

Please see my comment downthread.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#43

Post by stouset »

ScottBurson wrote: post #41
static_typed wrote: post #5

Serious developers take an interest in making things as simple as possible, ideally each piece have singular responsibilities, and no magic. Software engineering is hard, and it is heartening to see more people realise and promote awareness of some of the more dangerous anti-patterns we see in frameworks like Rails.

Let's get specific, because there's a point here that I think a lot of people are missing (including tptacek). ActionDispatch::Routing::RouteSet::NamedRouteCollection.add calls 'eval' on the property name passed to it. This invites exploitation. It also fails 'static_typed's desideratum of each component having a single responsibility. And I don't think calling it "magic" is too strong. I don't know to what extent th…

I consider myself an expert in Ruby style and best practices. And I have never and would never write code that uses `eval`, outside of toy projects.

There are probably ways you can use it safely. But it's not worth the risk.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#44

Post by tptacek »

ScottBurson wrote: post #42
tptacek wrote: post #34

Earlier quoted context omitted.

I'm responding to the comment above, not the post.

I don't read the comment in question as picking on Rails either ("... frameworks like Rails", it says). Please see my comment downthread.

"Frameworks like every framework" seems like an unintended but accurate generalization that was worth pointing out.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#45

Post by astrodust »

ehsanf wrote: post #23

I believe over-engineering is also a culprit here. We had a similar situation in JSON handling in browser. Some over engineered feature allows custom objects to replace built in object for lists, allowing XSS through JSON parser. The solution was to make every REST API to start with a top level dictionary object. It just sounds arbitrary over engineering!!

Why can't there be a set of parsers for YAML, JSON, and XML that are tested, abused, and audited aggressively so that your interchange formats don't become attack vectors?

The current state of having a half dozen of each of these is complete chaos. Presumably nobody thinks they're accountable because everyone has the option of using another package instead if they're not happy, basically passing the hot-potato constantly.

Is there a non-Ruby project that has a good implementation of these worth studying?

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#46

Post by ehsanf »

amalag wrote: post #35

I also don't understand why all parsing of user input needs to have YAML turned on by default. All this stuff should be turned off. I think it is a fundamental design choice that has to be looked at.

We should call it "insecurity by default" (in contrast to insecurity by design). A major problem is that nobody takes responsibility or pays attention for default choices.

A ton of packages have default choices that are inherently bad/insecure (mail servers listening on all interfaces by default, SSH servers accepting root login by default, and so on). Packaging is just as important as development.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#47

Post by alinajaf »

sanderjd wrote: post #29
alinajaf wrote: post #24

Earlier quoted context omitted.

> they mean that Rails has drawn that line quite far toward the abstraction side, which is true Can you give a concrete example of this?

Yes. When you write a controller action, you have access to what appears to be a hash called `params`, which is a representation of the parameters that the user passed in their request. It could be GET query parameters, or POST parameter data, or a parsed representation of JSON data. All of those different mechanisms are abstractions that Rails transparently provides. There are perhaps better examples in ActiveRecord…

> Yes. When you write a controller action, you have access to what appears to be a hash called `params`, which is a representation of the parameters that the user passed in their request. It could be GET query parameters, or POST parameter data, or a parsed representation of JSON data.

So in ActionController::Metal (a superclass of ActionController::Base) you have:

   def params
     @_params ||= request.params
   end
which is simply a case of a method in a super class being called in a subclass in your controller action. The rack request object in turn provides:

   def params
     @params ||= self.GET.merge(self.POST)
   rescue EOFError
     self.GET
   end
The Rack request object itself does the things you describe with GET/POST in each respective method.

A few points:

* It took me about 25 seconds to find this code.

* Making a method available in a subclass doesn't strike me as magic, its just inheritance.

* Aside from the aforementioned inheritance "magic", the rest of the things you described come from Rack, not Rails.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#48

Post by alinajaf »

tedunangst wrote: post #33
alinajaf wrote: post #24

Earlier quoted context omitted.

> they mean that Rails has drawn that line quite far toward the abstraction side, which is true Can you give a concrete example of this?

I would not expect rails to parse random snippets of xml fed to it just because it can. Not even security related. If I decide to move from rails to django, now I have to wonder if somebody out there decided to talk to my app using json instead of regular get/post, and deal with migrating/breaking that client.

> Not even security related. If I decide to move from rails to django, now I have to wonder if somebody out there decided to talk to my app using json instead of regular get/post, and deal with migrating/breaking that client.

If you didn't publicly document the fact that you accept a particular format you have no obligation to continue supporting it.

Your point about the XML stands though, that sucked!

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#49

Post by alinajaf »

kyllo wrote: post #39
adamors wrote: post #21

Earlier quoted context omitted.

> Code which is executing a whole bunch of stuff behind the scenes without the user of that code (the web developer) being aware But isn't this the whole point of a framework? That stuff gets done for you so you don't have to write everything from the ground up? Plus if you want to know exactly how everything is done, you can see it for yourself. As we all know Rails is open source and the code is perfectly readable…

>As we all know Rails is open source and the code is perfectly readable for anyone that knows Ruby. I beg to differ on the readability of the Rails source code. Even for someone with a strong command of Ruby, there are a lot of layers of abstraction, functions that call other functions that call other functions, and subclasses of subclasses of subclasses, spread across many different files in different places in the…

Can you point me to a concrete example of this? Rails as a codebase is decoupled reasonably well. As long as I take it in chunks, I can understand control flow through it reasonably well, and I probably have less ruby experience than you do.

Re: Why the Latest Rails Exploit Is Indicative of a Bigger Problem

#50

Post by sanderjd »

alinajaf wrote: post #47
sanderjd wrote: post #29

Earlier quoted context omitted.

Yes. When you write a controller action, you have access to what appears to be a hash called `params`, which is a representation of the parameters that the user passed in their request. It could be GET query parameters, or POST parameter data, or a parsed representation of JSON data. All of those different mechanisms are abstractions that Rails transparently provides. There are perhaps better examples in ActiveRecord…

> Yes. When you write a controller action, you have access to what appears to be a hash called `params`, which is a representation of the parameters that the user passed in their request. It could be GET query parameters, or POST parameter data, or a parsed representation of JSON data. So in ActionController::Metal (a superclass of ActionController::Base) you have: def params @_params ||= request.params end which is…

A few things.

The condescending attitude is not necessary. I never claimed that it is somehow impossible or prohibitively difficult to determine how things in Rails work - it is very possible and I do it myself all the time. What I claimed is that Rails has chosen abstraction over obviousness, and gave an example of a place where I believe that is demonstrated.

People usually inherit from AC::Base, not AC::Metal, which includes a number of modules, any of which may change the behavior of `params`.

You didn't take your expose far enough. As I mentioned in my post, `params` is an already-parsed representation of user data. You traced the call chain a couple of steps, but did not get to the point where parameters are actually parsed.

That's fine, I'm not at all interested in the specifics of how parameter parsing works, and I'm certain that you or I could go track it down when necessary. My point is only that it involves a number of layers of abstraction to get there - cataloging those layers is not a very compelling counterpoint.

Post reply on HN