The Design of Software (CLOSED)

A public forum for discussing the design of software, from the user interface to the code architecture. Now closed.

The "Design of Software" discussion group has been merged with the main Joel on Software discussion group.

The archives will remain online indefinitely.

SQL Injection?

I noticed Joel wrote about SQL injection in web sites today. Could the problem be solved in my own queries by just removing the ability to use the semicolon in an input field, so that someone couldn't end my existing query and start a new one of their own? Something like:

$query = "select personID, firstName, lastName
from people
where lastName = " . str_replace(";", "", $_POST['lastName']);
Tim Patterson Send private email
Wednesday, November 01, 2006
 
 
There's lots more vulnerabilities than just semi-colons - attempting to eliminate them all by doing input validation is geenrally not a good approach.  Use parameterized queries or stored procedures.
Mike S. Send private email
Wednesday, November 01, 2006
 
 
In PHP, use ADOdb.
Berislav Lopac Send private email
Wednesday, November 01, 2006
 
 
>>Use parameterized queries or stored procedures.

Seconded.

As the developer of the app, you know what actions should be being taken against the data at any given place in your app, so limit what the user can do to *that*.  There's almost no reason to allow Joe RandomUser to post what are effectively ad-hoc queries to your database. 

Plus, the database can optimise the handling of an SP vs the on-the-fly execution plan for random SQL.
a former big-fiver Send private email
Wednesday, November 01, 2006
 
 
As others have said, use parameterized queries.

I hate when developers start filtering out characters. It is the wrong solution to the problem. Why exclude these special characters if you could easily handle them AND be more secure? To me it is a bug to not be able to accept all characters. As someone pointed out in another post, imagine being someone named O'Brian and not being able to type in your name because a lazy programmer decided to just filter out the single quote character instead of solving the problem the CORRECT way!
anon
Wednesday, November 01, 2006
 
 
In perl and DBI, what do you mean by "parameterized queries"?

Is that when I put question marks into the query where the parameters go in the prepare function and then put the parameters into the execute function?

my $sth = $dbh->prepare("select * from table where name=?");
$sth->execute($NameInputFromUser)

This is what I usually do.  Is this bad?

??
My DBI vocabulary is not up-to-par
Wednesday, November 01, 2006
 
 
Yes.  They're also known as "prepared statements".

I haven't coded anything but prepared statements in over a year.  In PHP-land, with PEAR::DB or the mysqli binary module, there's no excuse for SQL injection vulnerabilities in 2006.

The only way an app using prepared statements can have an SQL injection vulnerability is if there's a bug in the prepared-statement layer (PEAR::DB, mysqli, mysql_real_escape_string, etc.) - and if that happens, you'll hear about it very quickly and the faulty component will probably already be patched.
Marco Arment Send private email
Wednesday, November 01, 2006
 
 
...and that "Yes" means that you are indeed talking about parameterized/prepared statements, not that they're bad.
Marco Arment Send private email
Wednesday, November 01, 2006
 
 
In java world if you always using preparedStatement.setXXX() then it should free from SQL injection.

However, for most application, we should spend time to manage the database permission, instead of just use single admin for everything.
Carfield Yim Send private email
Wednesday, November 01, 2006
 
 
Parameterized queries are a great solution in the ASP.NET arena but it is not too difficult to build a function that reviews user input looking for characters that need to be "escaped" for SQL (such as the ' in Joel's question) and also to look for and identify SQL key words.

I try and minimise user text input on all my web sites but from time to time you need to use some user originated text in an SQL statement - Usernames, passwords and such. A few lines of code to check the content are invaluable if you want to keep your site secure.
Mike Griffiths Send private email
Thursday, November 02, 2006
 
 
> "it is not too difficult to build a function that reviews user input looking for characters that need to be "escaped" for SQL"


You'd be surprised.  There are many potentially dangerous characters to each database engine that most people don't know about, such as null characters in the table's default charset (not necessarily single null bytes) in MySQL.  And each database engine has its own way of escaping quotes:  some use \', some use ''.

It's better to leave the escaping to the database itself by using prepared statements.
Marco Arment Send private email
Thursday, November 02, 2006
 
 
If you just want to escape strings easily in MySql use mysql_real_escape_string().

Prepared statements have the advantage that the query is compiled once and should work faster for multiple inserts but if you use mysql_real_escape_string() you should be safe.
Event Horizon
Thursday, November 02, 2006
 
 
mysql_real_escape_string() - I chuckle every time I see that function.

Round 1 "Hey guys - use addslashes(), it will stop hackers from so-called 'SQL injection attacks', but those never happen.  Dont forget to trap for magic_quotes because that will break this function."

Round 2 "Oh wait, that function doesn't cover all your bases, use mysql_escape_string() instead.  By the way, we don't tell you how to handle magic_quotes with this function."

Round 3 "No, wait, that one doesn't work right either.  You should really use mysql_REAL_escape_string().  This one is the REAL deal guys, we promise guys... no really.  this one works!  Oh yeah, dont forget to trap for magic_quotes cause you'll need to call stripslashes() first."

What a hack job.  And people wonder why there are so many flaws in most PHP programs - it is a pain in the ass just to do a simple thing like escape your input.
Cory R. King
Thursday, November 02, 2006
 
 
"I chuckle every time I see that function."

Me too.  And I'm a huge PHP fan.  There's just so much bad teaching and so many bad programmers for PHP that it gets an unnecessarily bad reputation.

PHP as a *language* is great.  But it's misused to create buggy, insecure web apps more often than Excel is misused as a database.
Marco Arment Send private email
Thursday, November 02, 2006
 
 
You can set up an account limiting to "select" only. Whenever there is possible SQL rejection, use this account to do select work.
Burner
Thursday, November 02, 2006
 
 
But SELECT-only privs doesn't solve authentication hacks.

Take your basic nonhashed login query with a username and password input:

SELECT * FROM users WHERE name = '$username' AND password = '$password';

If I can inject whatever I want as $password, I can use the following value to sign in as whatever username I typed in:

' OR 1 = 1 OR '' = '

Then the query becomes:

SELECT * FROM users WHERE name = 'Admin' AND password = '' OR 1 = 1 OR '' = '';

...and I'm logged in as Admin.
Marco Arment Send private email
Thursday, November 02, 2006
 
 
To the person that asked about perl DBI:

Yes, using either the '?' place holder or named place holders makes your query "parameterized." In the DBI even if the database doesn't support place holders the drivers usually do.

The '?' in your query gets replaced by the value you specify but that value is escaped according to the rules of your database, just like $dbh->quote('ffoo') does.

Parameterized queries (if supported by your database) may also provide a speed advantage. Most databases cache prepared queries, so if your query always looks the same ( "select a,b,c from foo where c = ?" ) it will always be prepared and cached as the same query no matter how many times you run it and with how many different parameters.

If you have "select a,b,c from foo where c = 'X'" and
"select a,b,c from foo where c = 'G'" they will be considered two different queries and will be prepared and cached as such. Additional similar queries will not hit that cache unless they are identical. So you ncurr the costof preparation on every query.
Clayton Scott Send private email
Friday, November 03, 2006
 
 
Clayton is correct.  To add to his information MOST RDBMS's support parameterized DML.  So even if you are trying to be "generic" (groan) going parameterized will work with the majority of them.  (I don't know of any that it won't work with)
Jim Send private email
Friday, November 03, 2006
 
 
For Perl, the main exception is using DBD::Sybase to connect to SQL Server. In that particular combination, parameterized queries don't work.

Naturally, that accounts for a good chunk of my Perl DBI work lately.
clcr
Friday, November 03, 2006
 
 
I just ended up working on some of my older web site code, and it turns out I was at times using mysql_real_escape_string(), but only when I thought the field (usually 'text' or 'textarea' in a form) would have characters like an apostrophe needing escaping.

Like a dummy I wasn't seeing the other good reasons to use the function more liberally whenever I used form input for a SQL query value. (I'm not one of those exceptionally bright folks Joel goes on and on about trying to hire, heh..)

Thanks to all who posted a reply, I've learned a few things.
Tim Patterson Send private email
Sunday, November 05, 2006
 
 
Never escape strings sent to a database.  As others have noted, INSIST on using parameterized queries.

Escaping strings is yet another form of "Enumerating Badness": http://www.ranum.com/security/computer_security/editorials/dumb/  Even if you think you have got it right according to the specs for your language, there is no way to prevent exploits due to bugs in your language or database implementation.

For example, what happens if a hacker attempts to embed "\n" into your input?  I think there is at least one database that will convert it to a newline, and then happily interpret anything that follows the newline... even if you have correctly escaped the backslashes, etc.  Unfortunately I can't find that link.
David Jones Send private email
Monday, November 06, 2006
 
 
There are some good whitepapers available at http://www.ngssoftware.com/research/papers/ that detail sql injection tactics.  They're old, but I think the techniques are still valid.  Escaping input strings isn't remotely close to being a solution.  And an injection attack can have far more serious consequences than most people are aware of.  The authors of those papers discuss an attack which contains no special characters that can transfer the entire contents of your database to the attacker's server.  I've never heard of anyone successfully doing this, but I also don't plan to be the first victim.
JP Send private email
Tuesday, November 07, 2006
 
 
I believe that the SQL Injection Bug can be eliminated by Only checking th Input Data not to contain any of the DML,DDL  statements that are supported by the Database.This can be easily checked by running a indexOf check on the input parameters for these and eliminating them if found.
For Eg: Noone will have a name with Select,delete or truncate ?  Further Discussion can continue based on oyur interest.
Satya Murty Send private email
Tuesday, November 07, 2006
 
 
Did you even read anything else that was printed here?

Haven't we already said time and again that filtering out keywords or special characters is not only going to fail, but it is a bug! There is no reason why someone should not be able to type the word "delete" into your application. Use parameterized queries and be done with it. It is not only the safe way to do it, but the easiest as well. It is far less code than what you are proposing.

Just insane! I can't believe that you people truly believe that you are the cream of the crop just because you hang around with Joel.
I'll be the ass
Wednesday, November 08, 2006
 
 
What about the performance benefits of parameterized queries? I'm an oracle junkie, but I know that it's expected that any modern DBMS do this. And, is would venture that any modern DBMS will also cache SQL statements and execution plans.

Tell me, if you have a query like this (in an imaginary language):

String s_sql =
 + " SELECT  1"
 + " FROM    USERS_TABLE
 + " WHERE  USER_NAME = " + s_username + " AND "
 + "        PASSWORD = " + s_password ;

Execute s_sql INTO n_result;

Then, for each time someone tries to login this whole statement must be parsed. That's a few CPU cycles * 1000s of executions, and these can add up quickly if you have even a Modestly sized application.

Now look at this:

String s_sql =
 + " SELECT  1"
 + " FROM    USERS_TABLE
 + " WHERE  USER_NAME = ? AND "
 + "        PASSWORD = ?" ;

Execute s_sql INTO n_result USING s_username, s_password;

Is this harder? No.
Is this cleaner? Yes.
Is this 100% safer? Yes.
How many times does this get parsed? Once, the first time.
Does it perform better? Yes.
Does it allow the database to do more useful work? Yes.

Why have a procedure to parse inputs (adding even more CPU overhead) when there is a RIGHT way to do things?!?
APH
Thursday, November 16, 2006
 
 
I had a asp.net website with a mssql backend and thought I could come up with a simple solution to injection by just replacing some special characters. But then the software tester got ahold of it and still was able to crash the site.

So I had to got serious. Change all text from the controls into hex text. Then the asp.net application only handles the user text in this hex text format. Update/Insert queries include a mssql function which does the translation back to text. When loading from the database a function translates the text back into hex text.

In this way one could theoretically put a binary image into the textbox and it would work fine.

This is extreme but it was easy and works very well. No input has been able to crash the website.
eric forsell Send private email
Monday, November 20, 2006
 
 

This topic is archived. No further replies will be accepted.

Other recent topics Other recent topics
 
Powered by FogBugz