Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Bad Code: Why is this dangerous? [duplicate]

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?

like image 563
Epaga Avatar asked Jul 16 '10 13:07

Epaga


People also ask

Why duplicated code is bad?

It's safe to say that duplicate code makes your code awfully hard to maintain. It makes your codebase unnecessary large and adds extra technical debt. On top of that, writing duplicate code is a waste of time that could have been better spent.

Is code duplication good or bad?

In computer programming, duplicate code is a sequence of source code that occurs more than once, either within a program or across different programs owned or maintained by the same entity. Duplicate code is generally considered undesirable for a number of reasons.

Is duplicate code a code smell?

Duplicated code is considered one of the worse code smells. Beyond blatant copy paste, there are subtle duplications like parallel inheritance hierarchies and repetitive code structures.

How do you convince a colleague that code duplication is bad?

Next time something needs to be changed in both files, make sure the task gets assigned to him. Better yet, see if you can convince him that since he thinks duplication is OK, he should own all such future tasks. THEN see how he likes doing the same work twice.


1 Answers

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."

like image 81
David Hedlund Avatar answered Sep 21 '22 14:09

David Hedlund