Over a million developers have joined DZone.
Platinum Partner

GitHub Hacked: How to Protect Your Code

· Web Dev Zone

The Web Dev Zone is brought to you in partnership with Mendix.  Discover how IT departments looking for ways to keep up with demand for business apps has caused a new breed of developers to surface - the Rapid Application Developer.

You should take a serious look at your application and write some tests, first thing Monday.

I would write integration tests with real data that attempt to exploit the issues that were exposed by the Github hack. Even if you’re sure of your code, sit down and write a few tests, just to be double-sure. Don’t do a code review, write some code that will tell you, 100%, whether you have problems or you don’t.

I see two major attack vectors.

Mass Assignment

Read Homakov’s post. If it’s not clear, read it again until it’s clear.

Given models Parent and Child where children belong to parents - can I post a parent’s ID to a form that updates a child and therefore change which parent a child belongs to? If so, you have a problem. Go fix it first thing in the morning in a systematic way, by writing a test that reproduces the issue, then by protecting the attributes with an attr_accessible method. This will filter out everything that’s not in the list when you call update_attributes. Make sure you just use this on all models, all the time.

A variation of this problem is garbage in, garbage out. This affects systems backed by NoSQL document databases. Make sure you aren’t writing random attributes that come from a form into your model. In a relational database you get an exception because the field doesn’t match the schema. In a document store you have just stored junk. It may be harmless or harmful, but you’d rather not find out the hard way.

We use a home grown hash map to whitelist attributes for historical reasons, but attr_accessible does the job just fine.

Identity Confusion

Whitelisting attributes only works when you actually don’t need to assign relationships. Do you pass an identity for a Widget as a parameter, maybe in a URL? Do widgets belong to different users? If so, write a test that ensures that a user that doesn’t have access to this Widget cannot modify it.

My recommendation is to use something like CanCan and to check authorization in a single layer. You spell out who can create/retrieve/update/delete models and enforce this with a single has_authorization_to?. We do this in our API layer systematically. We also learned to key off current_user as much as possible. So if you’re modifying widgets that belong to current_user, you won’t find a widget with a rogue ID by doing current_user.widgets.find(someone_elses_widget_id).

Dear Github

I still love you. This happens to the best people out there. Shameless plug for my former Team SHATTER, if you want a list. Move on and learn from it.

The Web Dev Zone is brought to you in partnership with Mendix.  Learn more about The Essentials of Digital Innovation and how it needs to be at the heart of every organization.


Published at DZone with permission of Daniel Doubrovkine .

Opinions expressed by DZone contributors are their own.

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}