Possible false positive?

[daveworth]

I've been tracking down what I think is a false positive in a client app. I've distilled what I think are the salient details but I could be missing something. I've added code that replicates the issue in my badapp's user model

False Positive?
Not an issue

I digging into the code it appears that the call to #include_user_input on (roughly line 199) of check_sql.rb is returning non-false for code that does include user input but should be escaped correctly.

It is my understanding that the #find_or_create_by_<attr> methods are safe to use user-input on. If they are not than the second example in my badapp linked above should also be warned about. My investigation in console (Rails 3.1) indicate that they are not vulnerable to a SQLi.

[presidentbeef]

Hi Dave,

The reason the first section of code warns and the second does not is because name is not determined to be user input.

But that's not the real issue. I agree that the #find_or_create_by_<attr> methods should not be checked for SQLi. This is apparently just an oversight on my part!

[presidentbeef]

I take part of that back...in find... methods, Brakeman is only checking the last argument (I assume because I thought this would be a conditions hash).