Bad Code: Why is this dangerous?

Question

Possible Duplicate:
Can I protect against SQL Injection by escaping single-quote and surrounding user input with single-quotes?

     String badInput = rawInput.replace("'","''");
     ResultSet rs = statement.executeQuery("SELECT * FROM records WHERE col1 = '"+badInput+"'";

Is there any way to do a "Bobby Tables"-like attack on this code?

Answer 1

Depending on the different steps along the way that all have to interpret the command, there may be some possibility to pass %27 (for instance) and have it act as a single quote, passing unnoticed through your replace.

But even if all such cases could be covered, and it was actually safe for this single question, it is lacking in that it cannot be uniformely implemented. Somebody else may come along and want to add AND int1 = var1, and notices that you have thought about SQL injection, so they just modify the code in the exact manner that you have

String badInput = rawInput.replace("'","''");
String badInteger = rawInteger.replace("'","''");
ResultSet rs = statement.executeQuery("SELECT * FROM records WHERE" +
 "int1 = " + badInteger + " OR col1 = '"+badInput+"'");

...only with integers it is no longer quotes you want to protect yourself from! Here, it is plain to see that anything could go wrong. So while that's a problem that requires somebody to implement it poorly, I think it's the biggest problem of the design - it only covers a narrow set of cases.

It will always be better to be able to just say "the following is a variable. whatever it contains, treat it as a value, and do not try to use parts of it as code and execute that code."

Answer 2

You might try:

badInput = "\\'; drop records; --";

in case your escape character happens to be set to '\'.

Answer 3

In MySQL if the NO_BACKSLASH_ESCAPES option is not set I believe it is possible to do.

\'); DROP 

Or something similar. Your code will double the '. The first ' will be escaped by the backslash and the second will close the string allowing SQL injection.

I usually prefer to stick to prepared statements to be on the safe side.

Answer 4

I'm not a security expert but won't char() effectively bypass your safety measures?

Eg: Getting everything from the records table

SELECT * FROM records WHERE col1 = char(39,97,39)

Eg 2: Writing info into files without single quotes

SELECT * FROM records WHERE col1 = concat(char(39),char(97),char(100),char(109),char(105),char(110),char( 39)) into outfile concat(char(39),char(97),char(100),char(109),char(105),char(110),char( 39))

Source here: SQL Injection Cheat Sheet

Answer 5

Because there only has to be one hacker that invents a way to bypass the ' replacements on a way that we cannot think of today. (A small bug in one of the layers to access the database???)

Answer 6

I recently introduced my brother to the prepare statement. Having implemented it on his current project, he found

• He could get rid of all the messy escaping
• he could get rid of all the messy string concatenation
• His code ran faster.

Why would anybody not use prepare?

Answer 7

That is prone and always inviting for an injection attack.

Answer 8

Maybe. Depends on what that replace method actually does, especially when it encounters unicode surrogate pairs or combining marks - and similarly how such pairs are handled by your database access technology. If replace works at a char level, then if an attacker supplies you with a valid high surrogate followed by a single-quote character, you might be substituting that single quote with a pair of single-quotes - in effect, appending a single quote after something that -might- later pass through an encoding and be interpreted as an invalid surrogate pair, leaving a naked single quote character.

Maybe.

Do you know enough about the unicode characteristics of that replace method and every intervening string handling library between your code and the SQL execution engine at the other end of the DB connection?

Are you feeling lucky?

Use a parameterized query.