March 7, 2012
This article was contributed by Nathan Willis
On March 4, a GitHub user attracted considerable attention with a controversial attempt to provoke the Rails project to change an easily-exploitable setting in Rails's default configuration. He did it by demonstrating the problem in the wild, granting himself commit privileges to the Rails master repository. Within a few hours, the hole was patched on GitHub and a fix deployed in Rails, but the debate rolls on about which parties are responsible, and about what other sites remain vulnerable.
Mass assignments
At the center of the trouble is Rails's
ActiveRecord::Base#attributes= method, which is widely used in
applications for updating all sorts of database records.
The method
accepts input about which fields (or attributes) of the record to change
concatenated together using a straightforward
&someAttribute=someNewValue syntax like that found in an HTTP
POST request.
By default, Rails allows any attributes to be
updated through this method, which means that attackers can change any and
all attributes of their choosing, simply by appending the command to
unvalidated form input. For example, injecting
&created_at=1955-11-05 into an HTTP request would overwrite
the value of an attribute which should be off-limits, but the update sails
through unimpeded.
Rails does offer a whitelisting macro called attr_accessible and a blacklisting macro called attr_protected, with which developers can restrict access to critical attributes, but neither is active by default.
This "mass assignment" situation has been regarded as a security weakness for years — it is discussed, among other places, in a 2008 article on Rail Spikes, which provides pointers to a mass assignment audit tool and advice for protecting one's applications — and points out that four of the five most popular Rails applications are vulnerable to mass assignment exploits.
On March 1, Egor Homakov opened
issue 5228 against
Rails (which is hosted at GitHub) calling attention to the problem, and
looking for a fix that would force developers to use
attr_accessible. The initial comment asks only for ideas:
I don't mean that it is Rails problem, of course not. But let's get it real
[...] how to avoid injections ? What should Rails framework do to force
people to keep their rails websites safe? Making attr_accessible necessary
field in model? What do you think guys.
It is after this initial bug report that opinion begins to diverge. The
issue was closed and reopened quickly by a core Rails developer, one
developer accused Homakov of trolling, and there was little in the way of
in-depth discussion. Then Homakov opened issue 5239 against
Rails on March 2, exploiting GitHub's own mass assignment vulnerability to overwrite the timestamp and peg the bug report as coming from the year 3012. In a comment, he apologized for the "inconvenience" and called the stunt "naughty testing" (though observing that he could use the exploit to close the issue himself).
Rails developer Xavier Noria then closed issue 5228 with the comment that "the consensus is the pros of the default configuration outweigh the pros of the alternative." Homakov protested that issue 5239 proved that the problem was widespread and that the Rails project should assume responsibility for offering secure defaults — at least by blacklisting certain obvious attributes, such as the creation-date overwritten in issue 5239. Noria replied that Homakov was "not discovering anything unknown, we already know this stuff and we like attr protection to work the way it is," and that "it is your responsibility to secure your application. It is your responsibility to avoid XSS, to ensure that the user is editing a resource that belongs to him, etc.."
On March 4, Homakov re-used the mass assignment vulnerability to grant commit privileges to his account for the Rails repository, and used it to add a file named "hacked" to Rails's master, containing the line "another showcase of rails apps vulnerability."
Responses
That commit is what was widely reported as the "hacking" or compromise of GitHub. Within an hour or so, GitHub pushed out a fix to the vulnerability, suspended Homakov's GitHub account, and published a blog announcement that it had "started an investigation."
To many commenters on the blog post and on issue 5228, that seemed like an overreaction, since it seemed clear that Homakov had only attempted to call attention to the security hole, and had not taken any destructive action. Later in the day, GitHub reinstated Homakov's account (at that point referring to it as a temporary suspension that had been made "pending a full investigation"), after concluding that he did not act with malicious intent. The second blog post also said that Homakov had privately reported a security flaw to GitHub on March 2, and that GitHub "worked with him to fix it in a timely fashion" — in contrast to the March 4 commit, which the blog post characterized as "without responsible disclosure."
It is not clear what the March 2 vulnerability report was; GitHub user
Max Bernstein said
he had exchanged email with Homakov, who said he wrote to GitHub about the vulnerability and GitHub had not responded. Nevertheless, the GitHub ship was righted relatively quickly.
Almost as quickly, Michael Koziarski committed a fix to Rails that requires developers to explicitly whitelist all attributes with attr_accessible — fixing the original issue.
Finger-pointing
Despite the speedy technical resolution, the debate over the incident continued to rage on — over whether or not Homakov had acted irresponsibly, whether the GitHub team had overreacted in suspending his account, and whether or not Rails or GitHub was ultimately at fault for the presence of the vulnerability in the first place.
It is the latter question that has the most far-reaching implications. After all, GitHub is offering a commercial service to the public, and has a responsibility to write secure code. But Rails, like any software framework, arguably exists to simplify the process of writing quality (in this case, secure) code. As GitHub user rainyday put it, "One of the reasons people use frameworks in the first place is because this type of thing is supposed to be done for you minimizing the chance of human error." Likewise, user Douwe Maan said "I'm disappointed that GitHub made such an obvious mistake" — but ultimately said Rails's defaults were to blame, which jeopardizes many other sites.
User Eric Mill commented on the GitHub-is-to-blame position, arguing that "The mechanism to secure Github is there without any code changes to Rails, which is how Github could fix it within minutes" — but also made the counterpoint, saying:
Rails has chosen a pattern which makes it more likely that even a foundational Rails site built by some of the most experienced Rails developers in the world is susceptible to unauthorized data entry. This is an obvious problem, and it reflects poorly on the Rails development team that they wouldn't take it seriously up until now. It's hard to get away from the feeling that as experienced Rails devs themselves, they simply could not empathize with the broader Rails community and chose to blame any vulnerabilities on the incompetence of individual developers.
The "disconnect" between the Rails development team and Rails application authors was echoed by others. As a commenter on LWN's own March 5 news item about the incident said:
If a language or platform has a misfeature which makes it hard to write secure code, it is hard for experts in that language to see why it's a problem. In principle, there is workaround XYZ which you should clearly use if you care about that stuff, but otherwise it is working as designed. The argument that *in practice* lots of programs end up with security holes does not carry the weight it should.
The whole affair reminded many of PHP's experience with register_globals. Although removed in PHP 5.4.0, in previous releases the register_globals directive was on by default, and allowed uninitialized variables to be injected into an application by many means — including HTML forms. The arguments for keeping it on were that many existing applications expected it to work that way, and that anyone who was concerned about the security implications could switch it off at will.
Ultimately, PHP bowed to public concern over the security of register_globals in the wild. Thus, although Rails did close the vulnerability after Homakov's stunt, GitHub user Karl Baron expressed surprise that the lesson needed to be learned again, saying in the issue 5228 comments: "Nobody here sees the irony in Rails redoing what PHP was ridiculed for for so long? Never. inject. user. input. by. default."
Finally, although the mass assignment problem may have been fixed in Rails's master, and repaired on GitHub, those fixes do not mark the end of the issue. If nothing else, the publicity surrounding the event has raised awareness — but certainly not everyone who has heard the news is free of malicious intent, and there are still scores of Rails applications vulnerable to the attack. Case in point: Chris Acky posted his own analysis of the events at Posterous (another Rails-based service), and shortly thereafter, comments began appearing with hacked timestamps, including one from Homakov stamped eight years in the past — and two others, apparently from someone else altogether.
The fact that the mass assignment vulnerability is
so widespread in the wild illustrates why some have mixed
feelings on whether or not Homakov's attention-grabbing stunt ought to be
regarded as heroically daring or recklessly bad. It does not change the
vulnerability of any site, but it greatly increases the likelihood of an
attack. Yet it is also a clear reminder that web frameworks should provide
sensible and secure defaults for their users — and that even popular
frameworks like Rails have a responsibility to learn from the mistakes of
others.
(
Log in to post comments)