1

Using Brakeman

One of the tools I learned about at Ekoparty was Brakeman, an open-source vulnerability scanner which does static analysis of Ruby on Rails applications’ code to find security issues. It’s a gem, so installing it is straightforward:

It’s as easy to use as cding into the directory of a Rails app and running  brakeman -o report.html . It’s really fast, just a couple of seconds for a middle size Rails app.

Brakeman is extremely suspicious, so after running it, a developer has to examine the output to check for example if the values arriving to a certain function are dangerous or not or if proper validations are being performed.

From a big list of warning types, these were the ones that appeared in my case (a very stable production app that was subject to similar analyses in the past):

  • SQL Injection: false positives. They were cases of string interpolation used as parameter of methods like .where  but the interpolation was done with variables defined in the code, not with tainted variables (with content read from user input).
  • Command Injection: one of these was a security issue. The code was supposed to run Rake tasks and looked like this: system("rake #{task}"). The fixed code, that doesn’t trigger the warning, is: system("rake", "#{task}").
  • Mass assignment: this is meant to prevent cases of instantiation with data received directly from the user, like: User.new(params[:user]. According the docs “Brakeman will warn on uses of without_protection”; it does even if the call is done with a literal hash with the parameter without_protection set to true: User.new({username: "jjconti", admin: false}, :without_protection => true). Issue filed.
  • Dynamic render path: according to the docs, “These warnings are often false positives, however, because it can be difficult to manipulate Rails’ assumptions about paths to perform malicious behavior. Reports of dynamic render paths should be checked carefully to see if they can actually be manipulated maliciously by the user”. For us, they were false positives.
  • SSL Verification Bypass: the application was connecting to a server using SSL but wasn’t checking the validity of the certificate. A man-in-the-middle attack could be performed. The Brakeman suggestion of changing http.verify_mode from OpenSSL::SSL::VERIFY_NONE to OpenSSL::SSL::VERIFY_PEER  was accurate.
  • File Access: this is a dangerous one. We have only one case of this. The code was something like: send_file payment.send(file_type).path. Despite the fact that it wasn’t a direct file access, before that line there was a check ensuring that file_type had only valid values: raise "invalid file type" unless file_type.in?(valid_types).

Cases like the last one were ignored to avoid too much noise in the report. For this purpose, Brakeman can read an ignore file. The file is a regular JSON file but the recommendation is to create and manipulate it using the tool itself:

Conclusion

Brakeman wasn’t able to find too many security issues is this app (2 out of 11 warnings of confidence level high) but the application is very stable, running in production for several years now and a similar analysis was performed on it in the past. In spite of this, the exercise of reviewing the code with another set of eyes (even if it was to finally discard a security issue) was enriching.

Picture by Simone A. Bertinotti

Comments 1

  1. Brakeman shouldn’t warn about User.new({username: "jjconti", admin: false}) and I can’t reproduce a warning like that. Can you please file an issue so we can fix the false positive? Thanks!

Leave a Reply