SQL Injection Filtering

Seth Brundle

I have been involved in a debate with someone about SQL Injection adn while I was brought up being under the idea that everything possible should be filtered including single quotes, etc and SQL keywords (SELECT, JOIN, UNION, WHERE, INSERT, DELETE, UPDATE, LIKE, DROP, CREATE, MODIFY, RENAME, ALTER, CAST). The belief I have is that all user unput that could be dangerous, is! My debating colleague believes that replacing single quotes and a few other main characters is sufficient.
Is replacing SQL keywords with an ASCII variant necessary? e.g. select becomes select
Leaving application languages aside, what filtering do you people use?

Merrion

You are right - everything goes in a parameter, not a quote...

e.g.
"SELECT [User Name] From Users Where Id = @Id"
then populate parameter @Id from the UI

not
"SELECT [User Name] From Users Where Id = '" & txtId & "'"

bonkey

Is replacing SQL keywords with an ASCII variant necessary?

Can you demonstrate a situation where your colleagues method of filtering is insufficient, but where yours will catch the problem?

If not, then one would have to ask what grounds you have for believing that the additional filtering is of any benefit.

Leaving application languages aside, what filtering do you people use?

I don't use filtering at all unless I have no other option - which is very very rare. As a general rule, I use Prepared Statements where the values which will be supplied as input are defined as parameters.

Injection attacks simply don't work against this technique.

If I do have to build statements piecemeal, I'm still inclined to build the SQL for a prepared statement, define the parameters, bind them, and proceed that way, because (again) it eliminates the possibility of an injection attack.

If I *have* to go with injection-prone techniques, then I would do the following:

1) Ensure the user cannot inject a string or statement terminator. This way they cannot inject something when supplying string-based input, as they cannot get "outside" the string delimiters that they are supplying a value for. I see no benefit in going further. If the user cannot break out of the string, they cannot inject. After that, I don't care what data they supply.

2) Ensure non-string-based data can be successfully cast to its expected datatype. Cast to this type, and if necessary cast the result back to a string.

3) Bounds-check all data (i.e. values are not too large for the DB field they are targetting).

jc

Seth Brundle

bonkey wrote:

Can you demonstrate a situation where your colleagues method of filtering is insufficient, but where yours will catch the problem?

If not, then one would have to ask what grounds you have for believing that the additional filtering is of any benefit.

As I said I was brought up in paranoia and was not suggesting that I or my colleague was in the right. There may be grounds for both and I was merely seeking other opinions on it. Am I being unnecessarily thorough filtering out (in a web application) characters such as ', <, >, etc. and SQL keywords?
However, even pages such as this recommend using filters ...

msdn website wrote:

Make sure your application looks for characters such as semicolons, equals signs, double dashes, brackets, and SQL keywords.

bonkey wrote:

I don't use filtering at all unless I have no other option - which is very very rare. As a general rule, I use Prepared Statements where the values which will be supplied as input are defined as parameters.

Injection attacks simply don't work against this technique.

If I do have to build statements piecemeal, I'm still inclined to build the SQL for a prepared statement, define the parameters, bind them, and proceed that way, because (again) it eliminates the possibility of an injection attack.

If I *have* to go with injection-prone techniques, then I would do the following:

1) Ensure the user cannot inject a string or statement terminator. This way they cannot inject something when supplying string-based input, as they cannot get "outside" the string delimiters that they are supplying a value for. I see no benefit in going further. If the user cannot break out of the string, they cannot inject. After that, I don't care what data they supply.

2) Ensure non-string-based data can be successfully cast to its expected datatype. Cast to this type, and if necessary cast the result back to a string.

3) Bounds-check all data (i.e. values are not too large for the DB field they are targetting).

I currently do these [along with the aformentioned filtering].

bonkey

I should clarify something...when I say I "filter", when dealing with strings I mean I escape any dangerous characters to ensure that they are only interpreted as part of the string I want them to be.

If you can't end the string - which requires an string-terminator or a command-terminator - then nothing else in there can be dangerous.

If you're using parameterised Prepared Statements, the argument is void, as there is neither need nor benefit in such precautions.

The reason I asked for an example is that I believe if you don't understand the threat, you can't have any faith in your solution. I know why string- and command-terminators pose a threat, as well as not casting user-input numerics. I wanted to see if you knew what the threat was that you were trying to filter against. I'm not saying it doesn't exist, mind...

jc

rsynnott

Simply escaping quotes should be sufficient, surely? No need to superstitiously **** with the user's data in the belief that it will make things better.

http://en.wikipedia.org/wiki/Cargo_cult

Of course, if you're using prepared statements, there is no need to do anything.

Seth Brundle

What I was thinking was where the application takes in data from a user (e.g. a username) and runs a dynamic SQL statement (no stored procs, etc.).
Assuming single quotes, double quotes, etc are escaped and numbers are checked, is it possible for unescaped SQL statements to manage to run? This was where we started bickering. I have always been led to believe that it was possible and therefore these should be filtered also.

rsynnott

kbannon wrote:

Assuming single quotes, double quotes, etc are escaped and numbers are checked, is it possible for unescaped SQL statements to manage to run? This was where we started bickering. I have always been led to believe that it was possible and therefore these should be filtered also.

Sorry, unescaped statements? What do you mean? Can you give an example? In ANSI SQL at least there is no need to escape double quotes. If you are doing things like "SELECT $column_name FROM atable" (or "SELECT "&column_name&"FROM atable" or whatever), though, that is dangerous, and you must enclose the column_name in backticks, then escape backticks within it. That one sometimes gets missed. (In general, however, that's a foolish way to write an application anyway, and you should consider just doing it properly)

bonkey

In ANSI SQL at least there is no need to escape double quotes.

Given the reference to MSDN, I'm guessing the OP is either working with MSSQL or Access. I'd guess the former.

In MSSQL, one can use either single or double quotes as string delimiters.

, double quotes, etc are escaped and numbers are checked, is it possible for unescaped SQL statements to manage to run? This was where we started bickering. I have always been led to believe that it was possible and therefore these should be filtered also.

If someone is leading you to believe it can be done, ask them how. If its a book / article, then check for examples/contact the author/etc.

Ultimately, if you've checked around, and no-one can give you an example of how its done, then you have to ask if perhaps you've been misled.

Yes, I know the MSDN article tells you that it should be done. However, it appears they don't explain why there's a threat.

And as I've said lots of times before...if you're using parameters, you don't need to do any of this. If you're doing this while using parameters, you're really just wasting your time.

jc

Seth Brundle

Grand then.
I don't recall where I first heard it but I have kinda used it since - perhaps seemingly unnecessarily.
Strange though, I was looking at the code for a forum I have (Web Wiz) which is ASP and Access or MS SQL and this too filters loads of stuff including SQL keywords - see attached. I must ask them why they use it.

rsynnott

Either they are actually allowing SQL to be passed in from user input and executed (and are thus idiots) or they are cargo-cult programming (and thus also idiots). They are needlessly corrupting their users' data; in particular, they are stripping double-quotes ENTIRELY.

VBScript-y people, for some reason, seem to have particular trouble with this stuff. This SEEMS to be an education issue, rather than a specific problem with the language.

Here's more:http://en.wikipedia.org/wiki/SQL_Injection

Merrion

Check out this article and especially the first comment/rebuttal...

democrates

When I'm checking user data I filter based on what's allowed rather than looking for dodgies and allowing everything else.
If the data is disallowed the request is rejected, I don't go casting it.

bonkey

democrates wrote:

If the data is disallowed the request is rejected, I don't go casting it.

Think about what you're saying.

How do you test if the a value is valid as a numeric without casting it to that numeric type at some point???

Once its cast there, theres' no reason to do additional testing on the original string - continue testing the cast value, and then use it in teh INSERT when you're finished.

jc

Seth Brundle

rsynnott wrote:

If you are doing things like "SELECT $column_name FROM atable" (or "SELECT "&column_name&"FROM atable" or whatever), though, that is dangerous, and you must enclose the column_name in backticks, then escape backticks within it. That one sometimes gets missed. (In general, however, that's a foolish way to write an application anyway, and you should consider just doing it properly)

This is what we were referring to - the debate was actually on a forum regarding ASP with Access.

Using the likes of (vbscript here)

"select * from table where txtfield = '" & txt_whatever & "'"

As you say this is dangerous but if the various chars & quotes are escaped somehow, then is it dangerous? If it is, why so? If it is then, should the SQL keywords be escaped?

As for examples of it working - I guess if I had examples of it working I wouldn't be asking. Im not 100% convinced that I was shown SQL Injection properly years ago (ignoring stored procs etc.) . However, i am still not 100% convinced that the above line cannot be hacked after escaping various the chars.

NutJob

Merrion wrote:

Check out this article and especially the first comment/rebuttal...

Obviously no one read to the bottom of the page on that one.

Personally i do use filters but only to log weird stuff for both curiosity and entertainment. Iv seen some real funny half assed attacks as a result and often brightens up my day.

the Stored procedure argument doesn’t always hold as concatenation can cause problems and still leave the system open.

Ill stick to paramaterised queries as the eliminate nonsense.

As for integer overflows etc parse everything and have a nice exception handler just dont pass everything off to sql dbs and pray


You understand no one is going to agree on this one but I program to the KISS standard (where possible)

Seth Brundle

I agree with most of the stuff there but it does take a long time to read and my eyes may have glazed over a bit. However, I don't think it answered my Q fully.

Using the insert example in www.w3schools.com/ado/ado_add.asp for example where it is direct user input (which is wrong, etc.) but lets say there was escaping of quotes etc. done between retreival of the forms values and the generation of the SQL statement.

Is SQL injection possible with this example?
If the SQL keywords were escaped in the forms variables also am I correct in presuming that it would stop injection?

democrates

bonkey wrote:

Think about what you're saying.

How do you test if the a value is valid as a numeric without casting it to that numeric type at some point???

Once its cast there, theres' no reason to do additional testing on the original string - continue testing the cast value, and then use it in teh INSERT when you're finished.

jc

Ok I was talking about php/mysql rather than asp/access, apologies.

I've been testing input data as follows:
Check the string length
Check the character type
Check the characters comply with the allowed regular expression pattern

It must pass each test before it moves on, if it passes all tests then I addslashes and put it into the mysql statement.

If I'm missing something here for the love of God please let me know!

Civilian_Target

Most of the time PHP's my_sql_real_escape_string will cover it - I also generally have a method that takes out keywords that could go in a "where" part of a staement and cause injection, but that's basicly just ='s signs, gt & lt braces and a couple of key words. Dunno if it helps, but it makes me feel better...

Enter

hm i beleve theres a tutorial or something on this at phpfreaks.com *o yeah im just too lazy to move my ass and type something here xD*

rsynnott

The paranoia is amazing... Why not just worry about the real issues and refrain from making up extra ones?

Seth Brundle

rsynnott wrote:

The paranoia is amazing... Why not just worry about the real issues and refrain from making up extra ones?

In my case its not so much paranoia but is just not wanting to be wrong!

rsynnott

kbannon wrote:

"select * from table where txtfield = '" & txt_whatever & "'"

This variety of query is safe provided that quotes are escaped in txt_whatever (assuming that it's logically safe; that is that it fits in with your model of what a user is allowed do). No superstitious grepping for 'INSERT' or similar need or should be done, unless you're trying to figure out who's trying to break in.

All that said, have you considered just using prepared statements? Unless the language or driver doesn't support them, they're a good solution.

Seth Brundle

It wasn't my SQL that was being discussed and I do try and use prepared statements in examples like the above.

democrates

rsynnott wrote:

The paranoia is amazing... Why not just worry about the real issues like what's the best chat up line and refrain from making up extra ones?

Ok, maybe no-one would misuse our systems, I'm giving up all my worldly goods and joining the promisekeepers anyway.

rsynnott

democrates wrote:

Ok, maybe no-one would misuse our systems, I'm giving up all my worldly goods and joining the promisekeepers anyway.

No, no, read my previous posts. By all means, clean up your user's input, but do it in a sensible way. This means escaping quotes. (There are certain specific circumstances where careful bounds checking may also be required). There's no point doing the equivalent of burning voodoo dolls of hackers; does you no good.

theexis

kbannon wrote:

As you say this is dangerous but if the various chars & quotes are escaped somehow, then is it dangerous? If it is, why so?

I think the main point was made earlier in the discussion. You should not be trying to "escape" everything that can possibly be wrong, you should actually just validate for an expected form of value using regular expressions etc. Unless you know exactly how your database engine code works you can't possibly escape everything (e.g. are you familiar with how ADO handles Unicode surrogate characters or precomposed characters which can fold down to characters within the ASCII range?).

NutJob

This still a weird one


for example if i allow a username field to contain as an example

characters A-Z a-z 1-9 im safe anything else gets throwen out


now if i allow non alphas im starting to enter dangerous terratory especially if i echo these back

everyones aware of the effect of | on unix and %20 being used

but what about <> being allowed and #

Straight away im in cross site scripting target zone

So you will probably end up doing both filtering and paramaterisation but it depends greatly on what your needs are.

I prefer to create an acceptable character list and avoid non alpha numerics where at all possible (but i still filter)

am i paranoid Hell yea but its worth it

rsynnott

NutJob wrote:

This still a weird one


for example if i allow a username field to contain as an example

characters A-Z a-z 1-9 im safe anything else gets throwen out


now if i allow non alphas im starting to enter dangerous terratory especially if i echo these back

everyones aware of the effect of | on unix and %20 being used

but what about <> being allowed and #

Straight away im in cross site scripting target zone

So you will probably end up doing both filtering and paramaterisation but it depends greatly on what your needs are.

I prefer to create an acceptable character list and avoid non alpha numerics where at all possible (but i still filter)

am i paranoid Hell yea but its worth it

Those are not SQL injection issues, though, and are better dealt with before output that on insertion to database. For instance, if you are outputting a text file, you will want '<' to stay '<'; if you are outputting an HTML file you will probably want it to become '<'. Where your database is (or will ever be) used for more than one output medium, you should really store the original data intact, and worry about these issues at output.

seamus

I've written a good few internal php/mysql sections for work over the past while, and if there's one thing I've learned, it's that people will input invalid data, no matter how obvious you make the input format. You'll see commas in numbers, double quotes in single-word inputs.

I try to catch most invalid input using regular expressions, and then escape everything that's input. I can be a bit lax when doing internal sites, but for external ones, everything gets escaped - even input from drop-downs can be faked.