Safely Passing Ruby Code in a Rake Task

Fri, Dec 4, 2015

I wanted to make a Rake task that would accept something like 1.month or 1.day as one of its arguments. The immediate tool I reached for was eval, and it worked like a charm. However, Code Climate (the tool we use for static analysis) complains about eval, and understandably so; the use of eval is dangerous and can lead to security vulnerabilities.

I’m using it inside a Rake task that is never exposed to third parties. Anyone malicious enough to run the task with a bad parameter to be evaluated would already have access to the system, and could wreak even greater havoc than by running malicious code through the task. I believe this is an accepted risk scenario, and Crystal (one of my colleagues who was reviewing my code) suggested that, since I had a valid point, I could turn off the Code Climate check for this particular instance.

I thought that was justified, but I also thought it was a slippery slope. I’m sure there are ways to pass in a string and have it dynamically interpreted, without having to expose the system to a security vulnerability.

Why is `eval` so dangerous?

Jimmy had a presentation on the YAML exploit and this is exactly what enables the exploit: a YAML payload is deserialized to an object that contains an #eval statement and then executed.

There is a good SitePoint article that goes in depth into how the exploit works.

ABV (Always Be Validating)

One way to work around the danger is to validate the string being passed.

args = { period: "1.day" } # this can be set through the rake task

def permit?(operation)
  permitted_operations = /^([1-9].(day|year|month|hour|minute)s?)$/
  !permitted_operations.match(operation.to_s).nil?
end

period = eval(args[:period]) if permit?(args[:period])

Here is the code I used to validate the parameters passed into eval. Since I know exactly what format the string would have, I can craft a simple regex to test for this.

However, since Code Climate does static analysis, it won’t give me cookie points for validating the input before passing it to eval. As long as there is a call to Kernel#eval, the Code Climate checks will fail.

A rose by any other name

We can instead use #instance_eval which is mostly the same thing, but instead of evaluating the string in the current context, it evaluates the string within the context of the object instance you’re calling it in.

class SandboxObject; end
period = SandboxObject.new.instance_eval(args[:period]) if permit?(args[:period])

#instance_eval is not necessarily more secure, since you can still pass in something that gives an attacker remote access if you don’t properly validate it. For example:

class SandboxObject; end
Sandbox.new.instance_eval("system('cat /etc/passwd')")

But since we’re now restricting the string evaluation to an instance of a sandbox object, you can sidestep a number of the more common attacks, such as redefining a method on a commonly used class.

Together with the input validation, this should be enough to mitigate against careless Ruby code being passed in as input. This also makes Code Climate happy, and I get to have my pull request merged.

TL;DR

I lied; there is no perfect way for a third party to pass in a string and have it safely evaluated. However, you can mitigate the damage with the following:

Validate your string parameters that are to be eval’d or executed.
Don’t turn off the Code Climate check; instead, look for another way to approach what you want to do.

Updates

Looi made a good point about sidestepping the regex altogether:

def validated_period(args)
  num, period = args.split('.')
  allowed_periods = %w(second seconds hour hours day days month months)

  if allowed_periods.include?(period) && num.to_i > 0
    num.to_i.public_send(period)
  end
end

Espen also makes a great point: the regex isn’t the issue, but eval is. Espen suggested using something like the Chronic library or .advance(period.to_sym => i) instead.