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?
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.
The latest Rails security flaw [1] is example of a common anti-pattern. Ned Batchelder wrote an awesome post [2] explaining how a similar issue may also exist in Python’s YAML parser [3]. Looking at these vulnerabilities, I am reminded of similar flaws in other frameworks and libraries.
The issue in each case is an abuse of extensibility. At first glance the idea is clever: allow for run-time execution of new code or binding of server-side variables without changing your compiled code, thereby greatly enhancing extensibility. For example, provide extensions to your Python YAML parser that allow you to create arbitrary objects and execute Python code; provide extensions to XML Template parsing that allows for arbitrary command execution [4]; or dynamically assign user-supplied parameters to server-side variables [5] (aka mass assignment) based on the parameter name. This kind of vulnerability is by design in contrast to many other by accident vulnerabilities. We called the mass assignment anti-pattern out [6] several years ago when doing a security analysis of the Core Java EE Patterns for OWASP.
I have a strong feeling we’ll see more vulnerabilities of this type, particularly with the rising popularity of standards like SAML that are built upon several layers of libraries implementing and extending complex specifications. These kind of issues can sometimes be hard to catch with an automated scanning technology, which means most organizations adopting the status quo of application security due diligence [7] will undoubtedly miss detecting some instances of extensibility abuse.
Security-minded developers can protect themselves by taking the following steps:
Turn off unnecessary extensibility in third party libraries and frameworks
Do not use untrusted input in libraries that provide broad extensibility, such as Apache’s Xalan [8] with extensions enabled.
Be vigilant about monitoring for and patching newly discovered vulnerabilities in frameworks and third party libraries. Wherever possible, sign up for security mailing lists or groups like Ruby on Rails Security [9]
Now that's an interesting idea: developer preferences for extensibility mechanisms in libraries. Anyone know of example libraries shipping with that capability?
An interesting question is why do developers prefer runtime extensibility over compile time extensibility? Runtime extensibility trusts whatever happens to come on the wire, compile time extensibility explicitly whitelists the exact set of functionality the server will support.
Hypothesis: developers don't like code generation. Partly because build systems are unreasonably difficult to tweak, partly because debugging tools are too crude to handle code generation graciously.
Yeah, that's a fair assessment - compile time code generation is unnecessarily painful in many popular languages. Often even if there is a facility for it built in (e.g. C preprocessor macros, C++ templates), it's less powerful and more complex than it needs to be because the designers of the language for some reason decided not to just let you use the same language at compile time that you use at runtime.
Doesn't this get into the issue of whether XML dependency injection is a good idea? All of the wiring and some logic is built up in some files that are outside of the build/compilation process. Doing "proper dependency injection" (quotes for sarcasm) to me has always made me worry that I've lost control of how my application works, and somehow based on the right XML/JSON/etc config magic it will work perfectly and meet all security standards.
Do your remote users have any control over those files? Probably not, or at least, probably not on purpose. The problem here is highly extensible systems that consume some user-sourced data and ultimately contain some sort of path whereby the user is essentially running a program with some set of capabilities they are not supposed to have. (Many of these attacks have resulted in arbitrary code execution, but weaker forms are possible, where perhaps you can only instantiate a class that results in some file being created on disk with arbitrary contents, or reads from it, or something.)
Many of these systems are written with the use case in the developer's mind where the data can be trusted to some degree, because perhaps they assume it's going to be read only by the program that generated it and only viewed by the user who already owns the computer, etc. In some sense the error only occurs when one takes this sort of software and then exposes it to the network, but it can be hard to realize when you... or somebody buried three layers deep in $YOUR_FAVORITE_FRAMEWORK... has done that.
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.
The problem isn't "magic" per se, and the problematic components already have very clearly defined singular responsibilities (an API doesn't get much simpler than "dump" and "parse").
Security problems often happen when two components that are perfectly safe in their original contexts are tied together in a way that their designers didn't intend. These parser problems are examples of that.
So it's actually a good thing that frameworks like Rails try to tie together many components for you "magically". The integration of all those little simple pieces is always a chance to screw up security. It's better to do that hard work once, collaboratively as a community, than to have every developer make those risky choices separately every single time.
It's the same cost/benefit analysis that explains why you should never roll your own crypto. Anybody can make a system that's "secure" enough to keep themselves from breaking it, but that's not nearly good enough. You increase security by increasing scrutiny.
Yes. No other security framework has ever had problems like Rails had. It's definitely not like enterprise Java frameworks that were designed from the outset to satisfy security demands from F-500 appsec teams have had repeated remote code execution flaws. And if they did, they definitely weren't as simple to trigger as "hitting a URL that maps to an embedded Java scripting language". No, only Rails has bugs like these.
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 bigger common issue.
There is no magic in Rails, it's just Ruby code. There will always be APIs that shouldn't be given untrusted data. When this happens anyway, calling it an "anti-pattern" does not mean you have perceived some kind of systemic fundamental flaw in the framework. That's just a transparently self-serving way for you to feel superior because after all you're a "serious developer".
I agree with you about the grating self-satisfaction of a certain vocal set of commenters, and have argued in the past myself that there is no "magic" in Rails, but I think what people really mean when they say "magic" as related to a chunk of software, is that it has so many layers of abstraction that its function is obscured.
All software deals in layers of abstraction, which makes it easier to reason about general function, but obscures specific function. All users of all software must make a constant trade-off between assuming abstractions are functioning properly and personally verifying their function, and all software must decide where the line between too little abstraction and too much obscurity should be drawn. When people talk about the "magic" in Rails, they mean that Rails has drawn that line quite far toward the abstraction side, which is true.
There are some abstractions that are a net positive for security, and some that hinder it. While I don't agree with the condescending attitude of your parent and the many similar commenters, who seem to think no software they've ever written or integrated with has ever had a security issue, I think it is fair at this point to say that the level of abstraction in Rails' handling of user data has been a hindrance to its security. I think that is probably also a fair thing to say about most pieces of software that have ever handled potentially-malicious user data.
I would tend to agree, except that Rails can't be all things to all people. Experienced developers don't find it nearly as magical as most of the people being told to "just learn Rails" when they get interested in web development. I totally get that abstractions can obscure security issues, but some of the reactions I've been seeing to Rails having security issues are akin to a parent giving a child a dangerous toy without reading the instructions and then getting mad at the toy company when the child hurts itself.
I think we agree on all points. People with less experience with Rails see "magic" where people with more experience see abstractions with all the trade-offs implied, and importantly, know how to dig into those abstractions to understand what is really going on. But that digging in process tends to be spurred by surprising behavior, rather than security consciousness.
I like your analogy to toys and children, except that the danger of these toys is not immediately apparent to adults either. But the toy company has been very prompt and transparent in finding ways for people to avoid the danger once it becomes apparent.
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, but that one seems more relevant.
> 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.
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.
As always in software, there are lots of options. Here are two:
1. Provide the action with the raw request body as a string. It is then solely responsible for any and all parsing.
2. Provide the action with the raw request body as a string, but provide standard parsing classes that the action can use depending on what they are specifically doing.
Both of those solutions would make the parsing code less abstract than the Rails solution, with different trade-offs.
I believe this entire thread started because I claimed that Rails has chosen to be further toward the abstraction side of the scale. I can't tell if you really think I am wrong about that, or if you merely think that the choice Rails has made is the right one. If it's the latter, I agree with you, and never claimed otherwise. But it is a choice, and it has trade-offs.
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!
magic is just a shorthand for "code executed automatically, behind the scenes, based on some reasonable assumptions". Rails does a whole lot of this type of magic, as does basically every framework.
this isn't a good analogy. Rails is a software development framework whose users are programmers. iTunes is a media player whose users are non-technical. in other words Rails is a tool while iTunes is a finished product. you wouldn't compare a hammer with a cabinet, would you?
if zero-framework development is a hammer, then development with a framework is a power-tool combination drill/hammer/driver. the power-tool does so much more then the basic tool that it feels "magical". its not a meaningless term then. its a description of the power of the tool.
I thought the magic term had something to do with explicit vs implicit. Some framework just don't call that much code behind your back without explicitly calling stuff.
Rails is more oriented in a "convention over configuration" mindset, in this sense it might be more "magic" than others.
"Magic" code is code that gives little or no indication that it's being executed in the contexts that are affected. Think of this as the "locality" of code. Do the names and operations being referenced in the current context completely describe the functionality that is being executed? Then the code has high locality. Magic code is code with low locality.
In the context of dynamic languages, "magic" does not mean someone casting spells. Of course it is just ruby code. It is magic ruby code. Code which is executing a whole bunch of stuff behind the scenes without the user of that code (the web developer) being aware.
Pointing out that doing really bad things like this is bad is no more transparently self-serving that you claiming "oh rails is totally fine and doing stupid shit is cool because its not really magic". Yes, it is a systemic fundamental flaw in the framework. The entire framework is built from the ground up on the idea that doing this kind of nonsense is good.
> 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 for anyone that knows Ruby.
>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 folder/project structure. It's quite difficult to figure out what is actually going on when you look under the hood of Rails, because it's complicated to the point where it's "indistinguishable from magic" until you have spend a lot of time with it.
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.
First of all let me make it clear that I'm not against all "magic", but let me address this:
> 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?
There is a vast difference between a framework that does stuff for you when you explicitly ask it to, and where what happens is fairly transparent, and a framework that does stuff for you because of code that without knowledge of the framework looks fairly "inert".
E.g. calling methods to define attributes and attribute-accessors on a class that maps to a database is not magic (though depending on how extensive, you might argue parts of what a specific ORM does in that case is): The code makes it plain that something will happen, and unless the naming of the method is atrocious, a developer will have a good shot at guessing roughly what from the code itself. You can make a framework that does a lot for a developer while still being explicit like this.
Automatically defining attributes pulled from the database based just on including a module, or inheritance, on the other hand, is well into "magic" territory: Not only might it not be clear it's going to be pulling stuff from the database and define lots of attributes for you, you don't know what it will define at all without looking at a schema that might not even be available together with the code, and that might change independent of the code.
It's not an either-or, but a sliding scale, and what is magic to some will be plain and obvious to others (the Arthur C. Clarke quote springs to mind). E.g. "everyone" who knows Active Record will know at least the basics of what Active Record will do to your class, and so won't be surprised by most of the method that to another seemingly magically materialize on it.
The trade-off is how far you go before the surprises to your target audience become too many.
I want my framework to do the boring stuff I don't want to do myself. That doesn't imply I want it running off and doing other things I don't even know about.
No, that is not the point of a framework at all. I am absolutely shocked that you would present it as though the options are "blindly do stuff automagically" vs "write everything from scratch". A framework is more useful if I control it, not less useful. If you want to give me the ability to parse yaml from GET params, then give me a
function to do it with. Don't just do it all the time in case I might want it. You don't need to automatically run a feature in order for the feature to exist, be used, and be useful.
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.
weren't some of the security holes found in the json & yaml libraries? Those weren't rails specific and could have just as easily happened in a Sinatra-like framework too.
