#358 Brakeman

Below is a screenshot from an application that has a number of security vulnerabilities. Finding these can be a bit of a challenge so in this episode we’ll use the Brakeman gem which can alert us to common security problems.

Brakeman will scan the Ruby code in a Rails app and look for common problems that might represent a security issue. It isn’t foolproof but it definitely helps to track down some nasty vulnerabilities so we’ll use it in our app to see what it can find.

Getting Started With Brakeman

To use Brakeman we’ll first have to install it, which is done in the usual way.

$ gem install brakeman

If you’re using rbenv you might need to run rbenv rehash to access the brakeman command after the gem installs. Once the gem has installed we can run the brakeman command from our Rails app’s directory. This will analyse the source code and print out a report.

$ brakeman
[Notice] Detected Rails 3 application
Loading scanner...
[Notice] Using Ruby 1.9.3. Please make sure this matches the one used to run your Rails application.
Processing application in /Users/eifion/store
# Rest of output omitted.

Brakeman has found several issues here but before we start looking at them we’ll generate an HTML version of the report instead. We do by using the -o option and if the filename passed to it ends in .html an HTML report will be generated.

$ brakeman -o brakeman.html

At the top of this file is a list of the checks that Brakeman has performed. This is followed by a summary of the results.

The most interesting part of the report is at the bottom. Here’s we’ll find a list of the issues that were found.

The first column here is a confidence rating which tells us how sure Brakeman is that its found a security issue. We can also see the location and type of the issue and a detailed message about it. As we’re looking at the HTML version of the report we can click on any message to see the source code surrounding the problem.

Fixing The Issues

We’ll go through the list of issues one at a time and fix them. We’ll start with the second one in the list. It’s message tells us:

All versions of Rails before 3.0.14, 3.1.6, and 3.2.6 contain SQL Injection Vulnerabilities: CVE-2012-2694 and CVE-2012-2695; Upgrade to 3.2.6, 3.1.6, 3.0.14

This is a problem specific to the version of Rails that the application uses. Our app is running on Rails 3.2.5 but an important security fix was introduced in 3.2.6. This is a fairly recent change so it’s good to see that Brakeman is kept up to date with security fixes. To remove this warning we just need to update the version number in the gemfile.

gem 'rails', '3.2.6'

Once we’ve done this we’ll run bundle update rails to install that new version of Rails. If we run brakeman again now to regenerate the report we’ll see that the issue has disappeared. The next issue is in the UsersController’s create action.

Unprotected mass assignment near line 7: User.new(params[:user])

There’s a related problem in the model warnings section.

Mass assignment is not restricted using attr_accessible

If we’re ever unsure as to what a report is talking about we should take a look at the Brakeman documentation. There are various articles here including a whole section on Warning Types which will give us detailed information about the several different types of security issues that Brakeman reports, including mass assignment. This explains this vulnerability and how to solve it. In Rails 3.1 and newer we can easily fix this by adding a line to the application config file.

config.active_record.whitelist_attributes = true

This is enabled by default in new Rails apps but if we’re upgrading from an older version of Rails it can be easily forgotten and it’s good to have a reminder from Brakeman. When we do this we need to remember to add attr_accessible to any models that don’t have it, such as the User model Brakeman warned us about.

class User < ActiveRecord::Base
  has_secure_password
  attr_accessible :name, :password, :password_confirmation
  validates_format_of :name, with: /^\w+$/
end

When we regenerate the report now these warnings have gone. The next problem we’ll tackle is this one, in the ProductsController.

Possible SQL injection near line 3: Product.order("name #{params[:direction]}")

SQL injection issues should always be taken seriously. If we look in the ProductsController’s index action we’ll see the poorly-written code that’s causing Brakeman to complain.

def index
  @products = Product.order("name #{params[:direction]}")
end

We’re inserting a parameter directly into a SQL order clause here, which is a bad idea. We can fix this by checking the value of the parameter to make sure that it’s only set to either asc or desc and using that value in the SQL. Brakeman is clever enough to know that params values are dangerous but that the string value that we set isn’t.

def index
  direction = params[:direction] == "desc" ? "desc" : "asc"
  @products = Product.order("name #{direction}")
end

When we regenerate the report now that issue has gone. The last security warning listed is this:

Possible unprotected redirect near line 12: redirect_to((session.delete(:return_to) or root_url))

Brakeman has given this error has a weak confidence so it may not be a problem but it’s a good idea to investigate it anyway so that we’re sure. The documentation on Redirect warnings has more details about this time of vulnerability. If we redirect based on the value from a user-supplied parameter then we can be subject to phishing attacks. An evil site could be set up that looks identical to ours and the owners of this site could use this vulnerability to redirect from our site to theirs. If someone signs in through our application and is redirected to the evil site they could be prompted to enter their password again. To protect our site from this vulnerability we can use the :only_path option in our redirect which will require that the redirect takes place only within our application. In our application’s SessionsController we redirect based on the value of a session variable called return_to which is set from a parameter in the new action.

class SessionsController < ApplicationController
  def new
    session[:return_to] = params[:return_to] if params[:return_to]
  end

  def create
    user = User.find_by_name(params[:name])
    if user && user.authenticate(params[:password])
      session[:user_id] = user.id
      redirect_url = session.delete(:return_to) || root_url
      redirect_to redirect_url
    else
      flash.now.alert = "Name or password is invalid"
      render "new"
    end
  end

  def destroy
    session[:user_id] = nil
    redirect_to root_url
  end
end

To protect from this we’ll use that only_path option in the direct.

def create
  user = User.find_by_name(params[:name])
  if user && user.authenticate(params[:password])
    session[:user_id] = user.id
    redirect_url = session.delete(:return_to) || root_url
    redirect_to redirect_url, :only_path => true
  else
    flash.now.alert = "Name or password is invalid"
    render "new"
  end
end

When we regenerate the report now this warning has disappeared.

Our final warning relates to the User model.

Insufficient validation for 'name' using /^\w+$/. Use \A and \z as anchors near line 4

This tells us that we’re using insufficient validation in a regular expression. We’re using caret and dollar signs as anchors where we should be using \A and \z. This is because we’re using validates_format_of in the User model to checks the format of the user name. Users can fool this by inserting new lines into their username as the regular expression only checks the boundaries of a line and not the full text.

class User < ActiveRecord::Base
  has_secure_password
  attr_accessible :name, :password, :password_confirmation
  validates_format_of :name, with: /^\w+$/
end

To fix this we just need to alter our regular expression.

validates_format_of :name, with: /\A\w+\z/

When we regenerate the report again now we have no warnings at all. Does this mean that our app is fully secure? Unfortunately not, there are several things that Brakeman cannot detect so it’s a good idea to always be on the look out for new security issues. Brakeman is a great tool but it should be used as a way to find potential issues and not relied on to guarantee than an application is free from problems.

There’s a lot to Brakeman that we haven’t shown here. If we look at the options in README we’ll see that there are a lot of ways to customize the way that the report is generated.

If we want to run Brakeman as a Rake task we can use the --rake option. This will generate a Rake file in our application.

namespace :brakeman do

  desc "Run Brakeman"
  task :run, :output_file do |t, args|
    require 'brakeman'
    
    Brakeman.run :app_path => ".", :output_file => args[:output_file], :print_report => true
  end
end

This generated task serves as a nice example of how we can run Brakeman from within Ruby. We can pass in various options to really customize the report and make it consistent for our application. If we want to run this command though we’ll need to add the brakeman gem to our gemfile.

gem 'brakeman', group: :development

Another way to run Brakeman is to use Guard, which we covered in episode 264. With the Guard Brakeman gem Brakeman will be run automatically whenever we make a change to a file.