Thursday, March 11, 2010

Ruby on Rails Security Review: An Experience Report

Image credits to Wink on Flickr (creative commons)

I was reviewing a Ruby on Rails source code to see the security implementations they have so far. They are about to launch their product for the first release and wanted to ensure they have the most obvious things checked. So, in a sense it was not supposed to be a hacking job for me, rather to check if the most well known security measures are in place. This is what I looked into so far:

  1. Password: Password was encrypted using a salt. However, the default logger would log the password as they didn't use the filter_paramerer_logging method.
  2. Cross-Site Scripting: I was able to easily inject a script by just entering <script>alert('Script')</script> when I signed up to the system and every time it would open an alert window whenever I navigated to a new page! So, I recommended them to use <%= h %>. However, Rails 3 does a good job of making this a default.
  3. Authorization: I found the weakest measure in the implementation of Authorization. For an example, there is a calendar in the web app where one can add/remove events. I found that any logged in user, not necessarily the event owner, could change/remove any calendar event. This was a shocker. Next, I found this same thing happening to the core models as well. The catch here is, they had a filter that checked if a user was logged in, but they didn't check if a user has rights to modify an instance of the object. For example, there is a project model, that can only be modified by the project owner. However, this per object ownership was not authorized and it was a huge potential security bug in my opinion.
  4. File uploads: The app was designed to upload the files to a folder underneath the public folder. Which means, if the rails server was down, apache would serve the files directly to the user bypassing whatever security measure was taken inside the app.
  5. PRG violation: This is a good idea to follow a post-redirect-get pattern when an object is modified through post/delete/put to ensure pressing the browser refresh button doesn't re-invoke the change. This wasn't done at some places which might end in multiple payments and such severe risks.
  6. Direct public release: I was a bit concerned that they wanted to go public release with their first ever release, even before having an alpha or at least in-house user. This is important because this application deals with money and credit cards. Trust is very important for such apps. So, I advised them to try this for some real works at home other than the "asdf asdf asdf...."(!) kind inputs. This will help them spotting some of the odd behaviors early and cause less embarrassment.
It was only a 4 hour assignment for me. Also, I was only limited to the source code and the test deployment that they have now. However, it seemed to me that, they might spend a few hours to fix the obvious errors and do some in-house real use before going to a public release.