Best way to go about sanitizing user input in rails

Question

I've read a lot about this and know there are many related questions on here, but I couldn't find a definitive guide for how to go about sanitizing everything. One option is to sanitize on insert, for example I have the following in my model

before_validation :sanitize_content, :on => :create
def sanitize_content
  self.content = ActionController::Base.helpers.sanitize(self.content)
end

Do I need to run this on every field in every model? I'm guessing the :on => :create should be removed too so it runs when updates too?

The other option is to sanitize when data is displayed in views, using simple_format, or .html_safe or sanitize(fieldname). SHould I be sanitizing in all my views for every single field, as well as on insert? Having to do this manually everywhere doesn't seem very railsy

Thanks for any help

Solution 1

TL;DR
Regarding user input and queries: Make sure to always use the active record query methods (such as .where), and avoid passing parameters using string interpolation; pass them as hash parameter values, or as parameterized statements.

Regarding rendering potentially unsafe user-generated html / javascript content: As of Rails 3, html/javascript text is automatically properly escaped so that it appears as plain text on the page, rather than interpreted as html/javascript, so you don't need to explicitly sanitize (or use <%= h(potentially_unsafe_user_generated_content)%>

If I understand you correctly, you don't need to worry about sanitizing data in this manner, as long as you use the active record query methods correctly. For example:

Lets say our parameter map looks like this, as a result of a malicious user inputting the following string into the user_name field:

:user_name => "(select user_name from users limit 1)"

The bad way (don't do this):

Users.where("user_name = #{params[:id}") # string interpolation is bad here

The resulting query would look like:

SELECT `users`.* FROM `users` WHERE (user_name = (select user_name from users limit 1))

Direct string interpolation in this manner will place the literal contents of the parameter value with key :user_name into the query without sanitization. As you probably know, the malicious user's input is treated as plain 'ol SQL, and the danger is pretty clear.

The good way (Do this):

Users.where(id: params[:id]) # hash parameters

OR

Users.where("id = ?", params[:id]) # parameterized statement

The resulting query would look like:

SELECT `users`.* FROM `users` WHERE user_name = '(select user_name from users limit 1)'

So as you can see, Rails in fact sanitizes it for you, so long as you pass the parameter in as a hash, or method parameter (depending on which query method you're using).

The case for sanitization of data on creating new model records doesn't really apply, as the new or create methods are expecting a hash of values. Even if you attempt to inject unsafe SQL code into the hash, the values of the hash are treated as plain strings, for example:

User.create(:user_name=>"bobby tables); drop table users;")

Results in the query:

INSERT INTO `users` (`user_name`) VALUES ('bobby tables); drop table users;')

So, same situation as above.

I hope that helps. Let me know if I've missed or misunderstood anything.

Edit Regarding escaping html and javascript, the short version is that ERB "escapes" your string content for you so that it is treated as plain text. You can have it treated like html if you really want, by doing your_string_content.html_safe.

However, simply doing something like <%= your_string_content %> is perfectly safe. The content is treated as a string on the page. In fact, if you examine the DOM using Chrome Developer Tools or Firebug, you should in fact see quotes around that string.

Solution 2

Because I always appreciate when I find the source of knowledge and code on any SO answer, I will provide that for this question.

Both ActiveRecord and ActionController provide methods to sanitize sql input.

Specifically from ActiveRecord::Sanitization::ClassMethods you have sanitize_sql_for_conditions and its two other aliases: sanitize_conditions and sanitize_sql. The three do literally the exact same thing.

sanitize_sql_for_conditions

Accepts an array, hash, or string of SQL conditions and sanitizes them into a valid SQL fragment for a WHERE clause.

However, in ActiveRecord you also have

sanitize_sql_for_assignment which

Accepts an array, hash, or string of SQL conditions and sanitizes them into a valid SQL fragment for a SET clause.

• Note that these methods are included in ActiveRecord::Base and therefore are included by default in any ActiveRecord model.

On the other hand, in ActionController you have ActionController::Parameters which allows you to

choose which attributes should be whitelisted for mass updating and thus prevent accidentally exposing that which shouldn't be exposed. Provides two methods for this purpose: require and permit.

params = ActionController::Parameters.new(user: { name: 'Bryan', age: 21 })
req  = params.require(:user) # will throw exception if user not present
opt  = params.permit(:name)  # name parameter is optional, returns nil if not present
user = params.require(:user).permit(:name, :age) # user hash is required while `name` and `age` keys are optional

The parameters magic is called Strong Parameters, docs here.

I hope that helps anyone, if only to learn and demystify Rails! :)

Comments

[Dave]

Thanks so much for the quick & thorough answer - very helpful! So I can basically allow anything to be inserted as long as I use correct query methods, but do I still need to use sanitize(field) or simple_format on every user-added field in my views? Otherwise couldn't someone enter some javascript or html that will be rendered as such in my view? Seems a bit of a hassle to manually sanitize every field in this way

[Dave]

Maybe I'm getting confused between sanitization for security vs stripping tags to prevent the user from formatting their content

[Paul Richter]

@Dave Oh ok, so as an example, you want to ensure that something malicious like <script type='application/javascript'>alert('You suck');</script> gets rendered as plain text rather than interpreted as html / javascript? If so, then you're safe to insert the content on to the page. I'll add a quick update to the answer for that.

[Paul Richter]

@Dave And regarding your first question in the comments: Correct, as long as you use the query methods, or at least parameterizing your SQL queries, you should be safe. Ultimately, whether you're talking about html/javascript safety, or SQL parameter safety, of course the best defense is to always consider what you're doing and think about whether its safe, and thankfully Rails is pretty helpful with that.

[Dave]

Makes sense, so just to confirm: the sanitize() function is not required for security purposes, and is only useful for, say, a wysiwyg field when I want to only allow certain tags - correct?

[Paul Richter]

@Dave Yeah, I think that would be an accurate example. Ultimately, what you said is correct; it is not required for security purposes because (as of the dawn of Rails 3, I believe) the ERB template automatically escapes your string content for you, so you don't need to use sanitize or <%= h(string_content) %> to ensure safety. However, I would also say that there's also nothing wrong with using it, if you truly wish to suppress all (or certain) html tags in your string content, but that's up to you, and your system requirements of course. Does that make sense (let me know if it doesn't)?

[Dave]

Got it, and thanks for probably the best answer I've gotten so far on SO

[Paul Richter]

@Dave Cool, I appreciate that. Glad to help.