Clearance 0.16.3 fixes a password reset vulnerability

The new release of Clearance works around the latest Rails SQL injection. Upgrade to Clearance 0.16.3 for the security fix.

gem 'clearance', '~> 0.16.3'

Background

In Clearance we generate a confirmation_token when you forget your password, and clear it when you successfully reset your password. In the controller we find the user like this:

@user = User.find_by_id_and_confirmation_token(params[:user_id], params[:token])

This approximately translates to this ARel query:

User.where(:id => params[:user_id], :confirmation_token => params[:token])

Normally this generates perfectly safe SQL:

SELECT users.*
FROM users
WHERE users.id = 1
AND users.confirmation_token = 'hello'
LIMIT 1

Exploit

If params[:token] is a list with one nil element, the generated SQL is closer to this:

SELECT users.*
FROM users
WHERE users.id = 1
AND users.confirmation_token IS NULL
LIMIT 1

That is, if you can get params[:token] to produce [nil] then you can become any user without a confirmation_token.

Prior to Rails 3.2.5, this URL would generate [nil]:

/users/1/password/edit?token[]

We catch this in Clearance now.

Fix

Upgrade to Clearance 0.16.3. If you are using Rails 3.2.5 or above then you do not need to upgrade Clearance to get this fix.

Acknowledgements

Thank you to Ben Murphy for bringing this to our attention in a professional manner, and to the Rails team for fixing it quickly.

If you gaze into nil, nil gazes also into you

Every developer runs into the dreaded nil object error: NoMethodError in Ruby, AttributeError in Python, and NullPointerException in Java. These errors are one of the largest sources of bugs. Even most static languages allow nil objects to silently pass as any type of object, and it’s just as easy in Ruby to let a nil object slip into seemingly well-factored code. If this is such a widespread and well-understood problem, why aren’t there best practices and solutions to solve it? There are actually many patterns and solutions for avoiding nil object errors, but you don’t see them too often in the Ruby community.

Don’t return nil, don’t pass nil

The most direct and bullet-proof way to avoid nil object errors is to avoid nil objects. Sometimes nil is passed around simply as a convenient “other” value or placeholder, and this is an abuse of the concept of nil. Finding and eliminating these abuses isn’t too hard, a program often needs to handle the concept of “nothing” in some way. Although nil is the most basic way to handle this case, it’s often not the best.

Handling the concept of “nothing”

As an example, let’s say you’re writing a software as a service application. SaaS applications frequently need to track which users are members of which projects. Some areas require administrator permissions, and you’d probably end up with code similar to this:

class User < ActiveRecord::Base
  has_many :memberships

  def admin_of?(project)
    membership_for(project).admin?
  end

  def membership_for(project)
    memberships.where(:project_id => project).first
  end
end

You can probably spot the issue with this snippet. If the user isn’t a member at all, membership_for will return nil and you’ll get a NoMethodError. We can easily change admin_of? to check for a missing membership, but that’s not really the problem with this code.

The problem with nil is that it’s hard to know when to expect it. As a developer coming onto this project, or as a developer coming back to a piece of code after a while, it’s hard to know which methods might return nil. A method named membership_for sounds like it should return a Membership, not nil. Unless you dive through every method you call (and every method those methods call, and so on), you can’t tell if a nil might be returned somewhere down the line. Rather than adding the nil check in this one place and continuing to program in paranoid fear, let’s look at some possible solutions besides returning nil instead of a Membership.

Exceptions

One alternative to returning nil is to raise an exception in the “nothing” case. The implicit contract of the membership method is that it will return a Membership object, so it makes sense to throw an exception in the cases that we can’t:

class User < ActiveRecord::Base
  class NoMembership < StandardError; end

  def membership_for(project)
    memberships.where(:project_id => project).first or
      raise NoMembership
  end
end

In this case, we don’t expect that you’ll ever call membership_for (or admin_of?) for a user that isn’t a member, so we can simply throw an exception in the unexpected case. The exception allows us to specifically handle the unexpected case of a missing membership (whereas NoMethodError is too generic to rescue), and it’s much easier to debug if it should happen in production.

Exceptions are useful when a method might not have a value to return, but nothing useful can be done unless a return value exists.

However, in a real SaaS, we’ll need to ask about permissions for users that aren’t members of projects, so raising an exception isn’t acceptable.

Special nil objects

Another option that I rarely see in Ruby code is the special nil object. A nil object can be written to handle situations that a method needs to handle “nothing” without returning an object that violates the implicit contract of the method:

class User < ActiveRecord::Base
  def membership_for(project)
    memberships.where(:project_id => project).first or
      NilMembership.new
  end
end

class NilMembership
  def admin?
    false
  end
end

With the NilMembership instance, the admin_of? method doesn’t need to worry about the return value. It will usefully respond to admin? whether or not the admin is a member.

Nil objects are useful when a method may need to return nothing and the returning method clearly knows how to handle the “nothing” edge case.

Unfortunately, this still won’t work for our real SaaS use case, because we’ll need to handle situations where a user is a member but isn’t an admin.

Maybe

Going back to our original code, now with an added member_of? method:

class User < ActiveRecord::Base
  has_many :memberships

  def admin_of?(project)
    membership_for(project).admin?
  end

  def member_of?(project)
    membership_for(project).present?
  end

  def membership_for(project)
    memberships.where(:project_id => project).first
  end
end

The member_of? method works fine, but admin_of? is back to raising NoMethodError for missing memberships. We still don’t want to just check for nil in that one client method, so let’s look at another option: the maybe pattern.

This pattern is a little more heavy-handed and requires some setup. Here’s a simple implementation:

class Object
  def maybe
    Some.new(self)
  end
end

class NilClass
  def maybe
    None.new
  end
end

class Some
  def initialize(object)
    @object = object
  end

  def get
    @object
  end

  def present?
    true
  end

  def blank?
    false
  end
end

class None
  class Unwrapped < StandardError; end

  def get
    raise Unwrapped
  end

  def present?
    false
  end

  def blank?
    true
  end
end

And here’s how we’d use it in our application:

class User < ActiveRecord::Base
  has_many :memberships

  def admin_of?(project)
    membership = membership_for(project)
    if membership.present?
      membership.get.admin?
    else
      false
    end
  end

  def member_of?(project)
    membership_for(project).present?
  end

  def membership_for(project)
    memberships.where(:project_id => project).first.maybe
  end
end

The idea behind the maybe pattern is to force client methods to unwrap the return value before using it. It’s impossible to be unaware that membership_for might return nothing, because it never returns an object that acts like a Membership. You could argue that this is a violation of the implicit contract based on the method’s name, but the return value at least prevents a developer from missing possible edge cases.

The maybe pattern is useful for situations that you need to return nothing but there’s no obvious edge case. In those situations, it makes sense to force consumer methods to check for nothing and handle edge cases on their own.

Return nothing without returning nil

Which pattern you choose (and there are other patterns out there) depends on your situation, but it’s likely you shouldn’t choose to return nil. Nil is a sad shell of an object, and it’s just not polite to hand a developer a nil when he or she asked for a Membership. Try helping your fellow developers out and find something else to return.

swallow nil

I’ve been using this for about a year and a half.

Abomination or legitimate tool for the box?

Usage

two dots or more. nil anywhere in the chain is an acceptable response.

campaign = swallow_nil { supporter.politician.campaign }

Definition

def swallow_nil
  yield
rescue NoMethodError
  nil
end

try()

It’s similar to try from ActiveSupport except for chains instead of individual methods.

Law of Demeter

Are you strict about Law of Demeter violations?

Or, do you find yourself with occasional chains like this that could benefit from swallow_nil() or try() or andand()?