How to fix user controlled method execution - Brakeman

Brakeman is a free static vulnerability scanner tool that helps you find security issues in your Rails application. In this tutorial, we will learn how to fix user controlled method execution security warning. Let’s consider an example.

Brakeman detects security warnings by statically analyzing the codebase for potential security vulnerabilities. Official documentation would give us potential suggestions on how to fix the security warnings. It is a good practice to keep an eye on security warnings in your application. This can be done by integrating brakeman with the CI/CD pipeline.

Consider a use case where we want to perform some changes to a User in the application. Let’s say, we have options to perform the following actions:

  • Activate user - which marks user as active.
  • Deactivate user - which marks user as inactive.
  • Archive user - which archives the user.

These options are displayed as a dropdown to the end user. The end user selects one of the action and hits Submit button. And any of the activity mentioned above is triggered on Submit button click.

Assuming, in case we have a single controller action update to process such change.

class UsersController < ApplicationController
  before_action :set_user, only: [:update]

  def update
    public_send(params[:status])
  end

  def activate
    @user.update(status: :active)
    # Do some more work required after inactivating user
    # ...
  end

  def deactivate
    @user.update(status: :inactive)
    # Do some more work required after deactivating user
    # ...
  end

  def archive
    @user.update(status: :archive)
    # Do some more work required after archiving user
    # ...
  end

  def set_user
    @user = User.find(params[:id])
  end
end

Note: Why these methods in controller? This is for illustration of the concept. This could be a piece of code in some service class or somewhere else.

Now, if you run brakeman security analysis over this controller, it will give a security warning as given below.

Confidence: High
Category: Dangerous Send
Check: Send
Message: User controlled method execution
Code: public_send(params[:status])
File: app/controllers/users_controller.rb
Line: 5

This is considered as security vulnerability, because if params[:status] is altered, it could lead to running any method dangerously.

Fix dangerous send method execution

This could be fixed by whitelisting method that needs to be called. For the case we described above, we can rewrite update method as given below.

def update
  case params[:status]
  when 'activate'
    activate
  when 'deactivate'
    deactivate
  when 'archive'
    archive
  end
end

This whitelists methods that are alllowed to be called. If params[:status] is altered, it will have no effect as allowed methods are only activate, deactivate and archive.

Similarly, we see occurrences where we derive class name dynamically based on the input parameter should be handled in similar manner.

class_name = params[:class_name]
model = class_name.classify.constantize

This should be avoided as params[:class_name] is an user input. Instead, use whitelisting strategy discussed above.

akshay

Akshay Mohite

Hi there! I am a Ruby on Rails & ReactJS Enthusiast, building some cool products at DTree Labs.

Read More
Buy me a coffee