About the Author

Chris Shiflett

Hi, I’m Chris: entrepreneur, community leader, husband, and father. I live and work in Boulder, CO.


Hacking Rails (and GitHub)

Hacker News exploded yesterday with news of GitHub being hacked. Wanting to know what all the fuss was about, I began with GitHub's side of the story:

A GitHub user exploited a security vulnerability in the public key update form in order to add his public key to the rails organization. He was then able to push a new file to the project as a demonstration of this vulnerability.

As soon as we detected the attack we expunged the unauthorized key and suspended the user.

My confidence in the clarity of GitHub's side of the story dissipated when I read one of the comments:

You didn't really "detect" anything. You were informed. It also wasn't an attack.

Not only did the "attacker" not do any actual damage, but he was continually ignored.

The author of this comment, Chris Acky, also made a more comprehensive blog post about the incident.

The "attacker" in question is Egor Homakov (his account has been reinstated), and he did in fact disclose the vulnerability a few days before he demonstrated it. There are a few facts worth noting up front:

  • This was not a public key security vulnerability as the title of GitHub's post suggests.
  • Egor is not a native English speaker, which might have made the potential impact of his discovery difficult to appreciate.
  • He was not actually ignored, but he was pretty firmly dismissed (citing a prior discussion).

Telling someone they're wrong only fuels their desire to prove they're right. It's not a huge surprise that Egor's next step was to demonstrate the vulnerability.

I'd like to explain the vulnerability, but rather than show you any code, I want you to understand the nuts and bolts, because it's extraordinarily simple. If you have a GitHub account, you can manage your SSH keys. The form to add a new key looks something like this (edited for clarity):

<form method="post" action="/account/public_keys">
  <input type="hidden" value="412e11d5317627e48a4b0615c84b9a8f" name="authenticity_token" />
  <dl>
    <dt>Title</dt>
    <dd><input type="text" name="public_key[title]" /></dd>
    <dt>Key</dt>
    <dd><textarea name="public_key[key]" /></dd>
  </dl>
  <input type="submit">Add key</input>
</form>

The authenticity_token is almost certainly an anti-CSRF token, so it doesn't complicate this exploit at all.

All Egor did was modify his own form to add the following:

<input type="hidden" name="public_key[user_id]" value="4223" />

The user_id of the Rails project is 4223, so that's why he chose it. (He believes this is a Rails issue, and it's hard to argue that.) By sending along this user_id, his public key was added to another account. Yikes!

For those of you more familiar with PHP, imagine a feature like register_globals, but instead of injecting arbitrary form data into the global namespace, it injects arbitrary form data into the database. It might as well be called opt-in SQL injection, but even that's being too generous, because this is much easier to exploit than an SQL injection vulnerability.

Egor points out that this vulnerability is unique to Rails:

Only Rails app have this kind of bug.

Wanting to better understand why Rails refuses to fix this, I looked into mass assignment, the feature in question, and found a post from last year:

If you're using Rails and you want to be secure, you should be protecting against mass assignment. Basically, without declaring attr_accessible or attr_protected, malicious users can set any column value in your database, including foreign keys and secure data.

While it's unfair to expect Rails to prevent mistakes, this does seem like a clear case where it promotes insecurity. As one person commented:

Rails is all about conventions. Broken by default is not a good convention.

Yehuda Katz has proposed a solution that people seem to like. Here's hoping this event can help raise the bar for what's expected of a framework. When Zend Framework was first announced, I made a wishlist, because I think frameworks are perfectly suited to help developers write more secure code. Rails is no exception.

If you're interested in reading more about this, here are some links:

Mass assignment vulnerability - how to force dev. define attr_accesible?
The GitHub issue where Egor first discloses the vulnerability.
wow how come I commit in master? O_o
Egor's commit to Rails, demonstrating the vulnerability.
Responsible Disclosure Policy
GitHub's followup post that mentions their new responsible diclosure policy.
"Egor, stop hacking GH"
Egor's original post, describing vaguely what he has been able to do and citing the fact that he feels ignored.
i'm disappoint, github
Egor's second post, where he proclaims his love for GitHub and disappointment with their response, suspending his account.
How-To
Egor's final post, revealing the details of the exploit.
Comment by Max Bernstein
References an email conversation with Egor, where Egor claims that he emailed GitHub about the vulnerability and received no response.
How Homakov hacked GitHub & the line of code that could have prevented it
Gist from Peter Nixey that explains a single line of code that can prevent this vulnerability in your own Rails apps.

About this post

Hacking Rails (and GitHub) was posted on Mon, 05 Mar 2012. If you liked it, follow me on Twitter or share:

11 comments

1.Chris Acky said:

Nice explanation. I was tempted to explain how it was done in my original blog post but figured I should stick to the reason which drove me to write it in the first place. (Referring to : GitHub and Rails: You have let us all down. )

This exploit is still going to have an impact on lots of sites. Just look at the comments on my blog post, users have changed time stamps to make it appear as though they wrote 1000 years in the future.

This is such a trivial "hack" as well. Quite shameful.

Mon, 05 Mar 2012 at 16:46:09 GMT Link


2.Kartik Mistry said:

One of the most brilliant post on this issue so far! Thanks!!

Mon, 05 Mar 2012 at 16:59:33 GMT Link


3.Nate Klaiber said:

Excellent post and explanation.

I still say this is more of a Github problem than a Rails problem. Rails API clearly states (and has stated) that you need to watch for mass assignment as a security hole. You can't expect the framework to hold your hand for you, especially when people have many different database schemas they are trying to support. How could Rails possibly know your business domain and which fields to protect and which to allow?

It's also a developer issue, in that you may have different models constraints based on the context or visitor - in which case rules can be constrained or relaxed.

This is an issue of authentication, and I don't think it's up to Rails to solve the problem for everyone. Developers should be smart, just like we can't blame PHP for SQL injection or other attacks. *WE* have to be educated and cognizant of the security of our apps.

Mon, 05 Mar 2012 at 17:42:08 GMT Link


4.Antony Dovgal said:

Whatever was the reason for his actions, whether he was bored, shmored, ignored, too lonely and wanted a hug, I don't care - his actions are still absolutely irresponsible and dangerous.

He could have continued patiently explaining the issue to the developers, but he just blew up and went 'demonstrating' instead.

This is sooo Russian.

Mon, 05 Mar 2012 at 19:30:53 GMT Link


5.Károly Négyesi said:

He was not actually ignored, but he was pretty firmly dismissed ... by the Rails community. Not by github. There's a github post saying: "Two days ago he responsibly disclosed a security vulnerability to us and we worked with him to fix it in a timely fashion. Today, he found and exploited the public key form update vulnerability without responsible disclosure." https://github.com/blog/1069-responsible-disclosure-policy

After leaving in a sechole in their Rails app, GitHub acted professionally and responsibly.

Mon, 05 Mar 2012 at 21:49:49 GMT Link


6.Adam Scheinberg said:

I'm not a Ruby guy, but it seems to me that this is the equivalent of this in PHP:

<?php
 
foreach($_POST as $k=>$v) {
 
     $db->query("UPDATE table 
 
     SET ".$k."='".$v."'");
 
}
 
?>

If that happened in a PHP framework, the internet would be ablaze about how bad PHP is (even assuming the value was properly db escaped.). This is a major issue, and I'd bet a huge chunk of Rails sites are vulnerable.

Tue, 06 Mar 2012 at 14:23:42 GMT Link


7.Rory McCune said:

Interesting post. One thing to note though is that Mass Assignment isn't uniquely a Rails problem, other MVC frameworks can have it as well..

http://digitalbush.com/2012/03/05/mass-assignment-aspnet-mvc/

would be one example.

Also I was surprised that github had this kind of problem. Mass assigment should be a well known security issue in the rails community, there have been presentations mentioning it as an issue going back to 2007

http://www.slideshare.net/jweiss/ru...security-218035

Wed, 07 Mar 2012 at 16:59:50 GMT Link


8.Vance Lucas said:

I work with both PHP and Rails everyday, and I think the register_globals analogy is way off. This problem is NOT unique to Rails, and is in fact encouraged in many PHP frameworks as well.

The PHP equivalent would be something like this:

<?php
 
$post = Posts::get(42);
 
$post->data($_POST['post']);
 
$success = $post->save();
 
?>

So the issue is mass assignment of request data 1:1 to named fields in your database without manually specifying which fields should be updated via a whitelist or blacklist. It's not the same thing as global variables at all, and the comparison to it is rather disingenuous.

Wed, 07 Mar 2012 at 22:03:07 GMT Link


9. said:

I think you're confusing yourself. Github may host the Rails project but they aren't responsible for tracking and fixing its bugs. Just as it would be wrong to blame an app running on Lithium, CakePHP or Symfony for not responding to a bug reported on their bug trackers.

Also, check out the definition of detect. Then ask yourself if a pedant, quibbling over the professional tone of a public disclosure, truly causes your confidence to wane in a product/business. If it still does, there's always Sourceforge!

I fear that you have been too hasty with your blog post and, with respect as a long time reader, my confidence in *you* will wane should you not correct your errors.

Thu, 08 Mar 2012 at 01:02:36 GMT Link


10.Wil Moore said:

I had been hearing for days that the Rails issue was similar to PHP register_globals; however, the two issues are fairly exclusive.

See the post by Vance as his example is spot on.

http://shiflett.org/blog/2012/mar/h...ithub#comment-8

Something like this:

$post->data($_POST['post']);
 
$success = $post->save();

Is indeed used in many PHP frameworks; however, I should add that the more respectable frameworks will also promote filtering (normalization) and validation of the data from the $_POST superglobal.

Once validated, the filtered hash of values would be passed to the model...never the raw values. This is essentially your whitelist. Any (full-stack) framework that doesn't promote that type of security out of the box is a framework you can easily dismiss.

If you are using a micro-framework that doesn't concern itself with any of this, that is fine, but just know what you are doing because you need to implement those security features yourself.

One liner to whitelist your parameters:

// $whitelist is an array of keys to accept
 
array_intersect_key($_POST, array_flip($whitelist));

Keep in mind that the above code is just a whitelist. You still need to filter (normalize) and validate.

BTW, I love many of the concepts behind Rails, but this recent issue is yet another reminder of why I don't use it in production. Not trying to disrespect anyone that does (I've seen some pretty cool Rails deployments), but it just doesn't work for me.

If you want to get into Ruby and are looking for a sweet web combination, check out Sinatra (based on Rack) + Data Mapper (version 2.0 is in the works and is looking awesome).

Thu, 22 Mar 2012 at 04:18:33 GMT Link


Hello! What’s your name?

Want to comment? Please connect with Twitter to join the discussion.