0

I have where method in Model and i am calling it in controller.

def some_method
  test = Something::Model.where(params[:param1],
                                params[:param2],
                                params[:param2],
                                params[:param3])
  ..more code here..
end

After pushing my code to remote branch, jenkins started throwing brakeman related errors. After further investigation, i found that brakeman is throwing Possible sql injection error at line ``test = Something::Model.where(params[:param1] So after some research i found that i have to use ActionController::Base.helpers.santize so when i used it as follows, it didnt throw any brakeman error.

def some_method
  test = Something::Model.where(ActionController::Base.helpers.sanitize(params[:param1]),
                                ActionController::Base.helpers.sanitize(params[:param2]),
                                ActionController::Base.helpers.sanitize(params[:param2]),
                                ActionController::Base.helpers.sanitize(params[:param3]))
  ..more code here..
end

My question is, is this the right way to fix that error or there is better way?

Thanks for reading

jose1221
  • 243
  • 1
  • 5
  • 13
  • see this question and its answers: https://stackoverflow.com/questions/21886170/best-way-to-go-about-sanitizing-user-input-in-rails – amrrbakry Jan 31 '18 at 19:29
  • So you have an ActiveRecord model and you're sending raw data out of `params` to the database by calling `where` on that model? Sounds like you might want to revisit the design. – mu is too short Jan 31 '18 at 20:19
  • 3
    This scope makes no sense. What is `:param1`? Why is it the first argument? This is a problem. Sanitizing fixes nothing because that scope shouldn't be valid. The normal pattern is `where(x: param[:x])` and so on. – tadman Jan 31 '18 at 20:24
  • @tadman theoretically it's possible that he sends raw sql (may be he's rewriting phpmyadmin in ruby, who knows) - smth like `params[:param1] == "my_field1 = ? OR my_field2 = ? OR my_field3 = ?"` and all the others params are values for placeholders, e.g. `params[:param2] == 2; params[:param3] == 3`. This will result in a perfectly valid scope `Something::Model.where("my_field1 = 2 OR my_field2 = 2 OR my_field3 = 3")` – trushkevich Feb 02 '18 at 06:50
  • @trushkevich That's exactly why I'm concerned here. Putting parameters like that in directly is **extremely** risky. There is no safe way of doing it. – tadman Feb 02 '18 at 18:35
  • Sanitizing doesn't fix anything here. Can you give examples of the parameters you're sending? – tadman Feb 02 '18 at 18:36

0 Answers0