Is it a security risk using eval in the model?

[11155]

Hi,

If I want to ensure that someone has filled out the email section of a
form I can write this in my model:

validates_presence_of :email

I can also achieve (more or less) the same thing by writing:

validate do |applicant|
  applicant.validate_presence("email")
end

def validate_presence(arg)
  string = "errors.add(:#{arg}, \"can't be blank\") if #{arg} == \"\""
  eval(string)
end

My question: does the method using eval pose any kind of security
threat?

I know the above example is silly (redefining an existing validation
method), but it serves well as a simplified version of what I am trying
to do, without going into unnecessary detail.

Thanks in advance

[DK11 (DK)]

You are ok if you are eval’ing on something which is not user provided. The risk is if you are eval’ing something which is user input, which then would subject you to risk. Below I am assuming your arg is a field name which is something passed by your own code.

David

[11155]

Thanks for the reply.

Below I am assuming your arg is a field name which is
something passed by your own code.

Exactly. arg is simply the name of a field whose value I want to check.
This field is hard-coded into my program.

I don't want to execute any user generated input, rather just check to
see if the user has entered anything.

I am assuming that in this case I am on the safe side using eval.
Is that correct?

[11155]

David Kahn wrote:

You are ok if you are eval'ing on something which is not user provided.
The
risk is if you are eval'ing something which is user input, which then
would
subject you to risk. Below I am assuming your arg is a field name which
is
something passed by your own code.

However, eval is virtually never necessary in Ruby. 99% of the time (as
in the OP's case), you actually wanted send. I'll post an example later
if it would be helpful.

David

Best,

[Michael_Pavling]

I'd say it's not a particular security threat (especially if you make
the 'validate_presence' method private). But it does pose a
code-legibility and slight smell threat. If you feel you need to use
eval, you're probably missing some other way of achieving the result
you're after.

Obviously, you've deliberately contrived your example to illustrate,
but by the same token that there's no need to do the eval in the
example (because you could just execute the line in the string), in
your real use-case I'd be suspicious of what other ways you could
arrange the code to avoid the eval.

NOI HTH

[Michael_Pavling]

There's a good case for the other 1% using "yield" ...

[11155]

Jim Burgess wrote:

Hi,

If I want to ensure that someone has filled out the email section of a
form I can write this in my model:

validates_presence_of :email

I can also achieve (more or less) the same thing by writing:

validate do |applicant|
  applicant.validate_presence("email")
end

def validate_presence(arg)
  string = "errors.add(:#{arg}, \"can't be blank\") if #{arg} == \"\""
  eval(string)
end

Here's the eval-less version:

def validate_presence(arg)
  errors.add arg.to_sym, "can't be blank" if self.send(arg).blank?
end

Of course, since this is ActiveRecord, you could use self[arg] instead
of self.send(arg) in most cases, but it's not quite equivalent. (I also
took the liberty of using blank? .)

Best,

[11155]

Thanks for all of the answers, you have really helped me a lot.
I was also unaware of the existence of the method '.to_sym' which will
in itself solve several of my problems. (Thanks Marnen!)
I will try out your suggested code to replace 'eval' and post back here
if I have any further questions.
Thanks again!
Jim