Fixing security vulnerabilities (27th Dec 2013)

Steve Kenworthy edited this page Jan 3, 2014 · 5 revisions

Fixing security vulnerabilities (27th Dec 2013)

Several serious security flaws have been discovered which affect all prior versions of Fat Free CRM.

Impact

This includes the following:

  • several SQL injection attack points (via the homepage timeline and activity features - requires user to be logged in to perform an attack) CVE-2013-7225
  • leakage of user data including password hashes and salts (logged in users are able to view password hashes of other users via json requests.) CVE-2013-7224 and CVE-2013-7249
  • lack of support for cycling the Rails session secret (Users are now forced to run rake ffcrm:secret when setting up a new instance) see CVE-2013-7222
  • protect_from_forgery was turned off by default CVE-2013-7223

Releases

Please upgrade your instance immediately to the latest gem versions 0.13.0 and 0.12.1 or grab the latest code from the github master branch. Note: you will be required to run "rake ffcrm:secret" before the server will restart. This will generate a new secret token for you in config/initializers/secret_token.rb

For those who are unable to upgrade, please use the workarounds section below to patch your existing system.

Ongoing security work and reporting

We will be conducting a security audit over the next little while to review the codebase for flaws.

It's no secret that the FFCRM codebase is very old in parts and desperately needs an overhaul. For those who are willing, we'd welcome the help. If you build your business on this, we'd ask you to consider giving back time and effort. Currently, this is entirely a volunteer maintained project.

For those who do find serious flaws, for the time being, please report discretely to steveyken@gmail.com

Workarounds

For those who are unable to upgrade, the following procedures should be applied to patch your existing system. You can either patch your code by cherry-picking the following commits or follow the manual procedure further below.

git fetch origin
git cherry-pick 93c182dd4c6f3620b721d2a15ba6a6ecab5669df   # Strengthen case to generate unique secret token.
git cherry-pick a7fedbb36388bad0c0f32b2346481e0ea126dea6   # Ensure requests are protected
git cherry-pick cf26a04b356ad2161c4c6160260eb870a3de5328   # Add custom serializers for xml and json.
git cherry-pick 078035f1ef73ed85285ac9d128c3c5f670cef066   # Fixed sql injection in timeline method.
git cherry-pick d4b2de81a4d8c1b201482edcb2488ed9280a65fd   # Refactor activity_user to remove possible SQL injection points.

Manual updates

1.. Rotate your secret token (if you haven't already, i.e. don't use the default version that Fat Free CRM provides.) Note: this is true of any other rails project you 'clone' from a public source.

To do this manually, open irb and type the following:

require 'securerandom'
SecureRandom.hex(64)

Then paste the result into config/initializers/secret_token.rb.

Note: the main reason for cycling this in your own project is that the secret token should not be publicly visible. If it's checked in on your github fork, it is publicly visible. More reading here: http://robertheaton.com/2013/07/22/how-to-hack-a-rails-app-using-its-secret-token/

2.. Add protect_from_forgery to ApplicationController.

In app/controllers/application_controller.rb, change add protect_from_forgery to the beginning of the class. This will prevent many types of CSRF attacks.

class ApplicationController < ActionController::Base

  protect_from_forgery

  ...rest of file

3.. Add the following method to your user class (before the private declaration) to stop important details being leaked via json. This is particularly serious as it enables any logged in user to view the password hash/salt of any other user. (Note: this does NOT disclose plain text passwords but, regardless, is still very serious.)

In app/models/users/user.rb, add the following lines above the 'private' keyword.

  def to_json(options = nil)
    [name].to_json
  end

  def to_xml(options = nil)
    [name].to_xml
  end

In config/routes.rb, change

resources :users, :id => /\d+/ do

to

resources :users, :id => /\d+/, :except => [:index, :destroy] do

4.. Patch the following code in app/controllers/home_controller.rb

This replaces two methods that had SQL injection vulnerabilities with code that sanitizes user input properly.

  • Replace the timeline method with:
  def timeline
    state = params[:state].to_s
    if %w(Collapsed Expanded).include?(state)
      if (model_type = params[:type].to_s).present?
        if %w(comment email).include?(model_type)
          model = model_type.camelize.constantize
          item = model.find(params[:id])
          item.update_attribute(:state, state)
        end
      else
        comments, emails = params[:id].split("+")
        Comment.where(:id => comments.split(',')).update_all(:state => state) unless comments.blank?
        Email.where(:id => emails.split(',')).update_all(:state => state) unless emails.blank?
      end
    end

    render :nothing => true
  end
  • and replace the activity_user method with
def activity_user
    user = current_user.pref[:activity_user]
    if user && user != "all_users"
      user = if user =~ /@/ # email
          User.where(:email => user).first
        else # first_name middle_name last_name any_name
          name_query = if user.include?(" ")
            user.name_permutations.map{ |first, last|
              User.where(:first_name => first, :last_name => last)
            }.map(&:to_a).flatten.first
          else
            [User.where(:first_name => user), User.where(:last_name => user)].map(&:to_a).flatten.first
          end
        end
    end
    user.is_a?(User) ? user.id : nil
  end

Please note: these methods are slated for an upgrade soon. Whilst it's tempting to refactor more fully right now, the focus has to be on locking them down with minimal changes for you.