views:

127

answers:

6

I'm working updating some legacy code that does not properly handle user input. The code does do a minimal amount of sanitization, but does not cover all known threats.

Our newer code uses parameterized queries. As I understand it, the queries are precompiled, and the input is treated simply as data which cannot be executed. In that case, sanitization is not necessary. Is that right?

To put it another way, if I parameterize the queries in this legacy code, is it OK to eliminate the sanitization that it currently does? Or am I missing some additional benefit of sanitization on top of parameterization?

+2  A: 

There are many different reasons to sanitize and validate, including preventing cross-site scripting, and simply wanting the correct content for a field (no names in phone numbers). Parameterized queries eliminate the need to manually sanitize or escape against SQL injection.

See one of my previous answers on this.

Matthew Flaschen
In other words, sanitization may still be useful for other reasons not related to SQL injection?
Bruce Alderman
Yes, @aardvark.
Matthew Flaschen
+1  A: 

You are correct, SQL parameters are not executable code so you don't need to worry about that.

However, you should still do a bit of validation. For example, if you expect a varchar(10) and the user inputs something longer than that, you will end up with an exception.

Hugo Migneron
+4  A: 

Parameterized queries will help prevent SQL injection, but they won't do diddly against cross-site scripting. You need other measures, like HTML encoding or HTML detection/validation, to prevent that. If all you care about is SQL injection, parameterized queries is probably sufficient.

tvanfosson
+2  A: 

It's true that SQL query parameters are a good defense against SQL injection. Embedded quotes or other special characters can't make mischief.

But some components of SQL queries can't be parameterized. E.g. table names, column names, SQL keywords.

$sql = "SELECT * FROM MyTable ORDER BY {$columnname} {$ASC_or_DESC}";

So there are some examples of dynamic content you may need to validate before interpolating into an SQL query. Whitelisting values is also a good technique.

Also you could have values that are permitted by the data type of a column but would be nonsensical. For these cases, it's often easier to use application code to validate than to try to validate in SQL constraints.

  • Suppose you store a credit card number. There are valid patterns for credit card numbers, and libraries to recognize a valid one from an invalid one.

  • Or how about when a user defines her password? You may want to ensure sufficient password strength, or validate that the user entered the same string in two password-entry fields.

  • Or if they order a quantity of merchandise, you may need to store the quantity as an integer but you'd want to make sure it's greater than zero and perhaps if it's greater than 1000 you'd want to double-check with the user that they entered it correctly.

Bill Karwin
i hope those $columnname and $ASC_OR_DESC don't come from the user! these should be hard constants in your code, and do a full audit of the data path from constant through variable to SQL, to be sure it IS one of your variables. (easier if that path is very short, so it's only a parameter at a very low level, at the immediately higher level, it should be different function calls, without any parameter that would end in SQL code)
Javier
@Javier: Isn't it a common UI feature to allow the user to click on the head of a column to sort by that column? But yes, you would want to validate that the UI input corresponds to a column name, using a whitelist or a map of some kind. The value in the UI doesn't even need to be the literal name of the column, it could be a value that your code can associate to an entry in a list of valid column names.
Bill Karwin
+1  A: 

In short no. Input sanitization and the use of parameterized queries are not mutually exclusive, they are independent: you can use neither, either one alone, or both. They prevent different types of attacks. Using both is the best course.

Stephen C. Steel
+1  A: 

It is important to note, as a minor point, that sometimes it is useful to write stored procedures which contain dynamic SQL. In this case, the fact that the inputs are parameterized is no automatic defense against SQL injection. This may seem a fairly obvious point, but I often run into people who think that because their inputs are parameterized they can just stop worrying about SQL Injection.

Yellowfog