3

Possible Duplicate:
Best way to prevent SQL injection in PHP?

I have been doing some research about SQL Injection but I have some questions that I couldn't find answer to. Isn't it possible to prevent SQL injection attacks on string levels? What I mean is, can't we prevent it by;

  • Finding illegal characters before processing them through mysql queries?

    $postID = $_POST['id'];
if($postID contains characters)
    remove characters;
if($postID still contains characters)
    then exit;
else
    mysql_real_escape_string($postID); //just in case?
continue to do whatever you are doing...

Is it really necessary to use PDO/mysqli stuff? Is it sufficient to analyze your sql strings to be processed in mysql? Please keep in mind that I am not a PHP or MySQL expert while replying. I am someone who is trying to learn about them.

Community
  • 1
  • 1
  • 6
    Possible? In theory. Should you try? Never. You *will* forget something. – Andrew Barber Oct 30 '12 at 21:05
  • 2
    Escaping is usually much simpler than removing. Also, your removal algorithm might damage valid input that was mis-identified as an injection attempt. – Niko Oct 30 '12 at 21:06
  • Removal is BAD. You should always escape. You might remove 's from a piece of text, but it's quite reasonable to use them in text (notice I just did). – Philip Whitehouse Oct 30 '12 at 21:08
  • 3
    I have done the same when securing some older applications I wrote. An ID for instance is only numeric, so if you strip away everything [^0-9] then it should be fine. Its more tricky with text areas though... But for new code! No! – jtheman Oct 30 '12 at 21:08
  • 2
    No character is illegal. You must have misunderstood something very fundamental. Try to find out what it is and you will learn for programming life. – hakre Oct 30 '12 at 21:22

6 Answers6

4

Sure, you can protect against injection with mysql_real_escape_string($postID), as long as you don't mind a query every time you call the function.

PDO and MySQLi provide a lot more than just injection protection. They allow for prepared statements that can protect agaisnt injection without multiple calls to the db. This means faster overall performance. Imagine trying to insert to a table a user record with 30 columns... that's a lot of mysql_real_escape_string() calls.

Prepared statements send all the data at once along with the query and escape it on the server in one request. Mysql DB's support prepared statments, the old php mysql_ libraries don't support them.

Time to move on to mysqli or preferrably PDO--you'll never look back.

Ray
  • 40,256
  • 21
  • 101
  • 138
  • Does that really run a query? How does the PDO version work? I ask because I tend to use `$mysqli->real_escape_string();` – Philip Whitehouse Oct 30 '12 at 21:08
  • 2
    *Psst:* `mysql_real_escape_string` does not protect against SQL injection if you don't provide the link to the database. So your example in the answer *is* prone to SQL injection. Take care. – hakre Oct 30 '12 at 21:14
  • That may be better said as "mysql_real_escape_string **may** not..." - if your link is no different than the default you **should** be good. Even so, I also recommend using mysqli_* – Joel Mellon Oct 30 '12 at 21:16
  • @hakre I though it would fail if a connection is not available or specified with a 'false' – Ray Oct 30 '12 at 21:16
  • Okay then, another thing... What makes different **mysql_real_escape_string()** than **addslashes()** ? Which is more secure, if we ever consider using this "string method" ? –  Oct 30 '12 at 21:17
  • 1
    @sudopeople: Well, right. Even without using any function, it *may* protect against SQL injection - just depending on database configuration and what that string is ;) – hakre Oct 30 '12 at 21:18
  • @SertacBalcı: I think you are old enough to a) to read the manual (I'm sure you can read) and b) to reasearch your own (That's a little harder than reading, but related). But anyway, the information is already there: http://stackoverflow.com/search?q=difference+mysql_real_escape_string%28%29+than+addslashes%28%29 – hakre Oct 30 '12 at 21:19
  • @hakre I wasn't discussing anything, just asking another question. And maybe I didn't explain enough. What I want to know is **not** the difference between those functions, I actually mentioned it in the second part. As you _can_ see in my main question, I started with _"I have been doing some research..."_ so what about being just nice, not criticizing mockingly? –  Oct 30 '12 at 21:25
  • @hakre, that's a good point ;) – Joel Mellon Oct 30 '12 at 21:26
  • 1
    This question is getting out of hand. There are a million of these on StackOverflow. – Joel Mellon Oct 30 '12 at 21:27
  • @SertacBalcı: There is nothing *more* secure. Security is a process not an existing something. A function normally is secure if it does not have any flaws. However for secure coding, you need to use any function securely. Don't compare the functions, more important is your own code. If you are really concerned, read the source-code of PHP and do your analysis. – hakre Oct 31 '12 at 08:20
2

I would encourage you to use PDO (PHP Data Objects). It will help against SQL injection and should speed up queries. Also, your application becomes more abstracted from the database.

Something like the following:

$stmt = $conn->prepare("INSERT INTO table_name VALUES(:id, :firstname, :lastname)");
$stmt->bindValue(':id', $id);
$stmt->bindValue(':firstname', $firstname);
$stmt->bindValue(':lastname', $lastname);
$stmt->execute();
ajtrichards
  • 29,723
  • 13
  • 94
  • 101
  • Never using a method with the words "escape string" in it is a big step towards actually writing secure SQL. – tadman Oct 30 '12 at 21:40
0

I wouldn't encourage you to use the mysql_ functions anymore because they're deprecated. Check: http://php.net/manual/en/function.mysql-real-escape-string.php for more information on why the regular mysql_ has been discontinued. You could switch to mysqli really easy. It really isn't that complicated. Plus if you want to escape characters and you don't want to be replacing every mysql_real_escape_string for the new way of escaping, you could use this function: 

function mysql_string_safe($stringtoclean)
{ 
    //In this case, 
    $safestring = mysqli_real_escape_string('your_handle_here',$stringtoclean);
    return $safestring;
}

So then you would replace mysql_real_escape_string for mysql_string_safe. Mysqli is really similar to mysql_ but It's more secure. I've switched to it and it didn't take too long, it was easy since it was just replacing stuff. Connecting to the database is different too. On the other hand, PDO has support towards different database drivers but if you want to save time and you're not going to switch database drivers, just use mysqli.

Ignacio Belhot Colistro
  • 339
  • 1
  • 13
  • 2
    This is taking bad advice, wrapping it up in a perplexing function name, and creating a bundle of awful that hopefully will never hit a production application. Please, don't do this. Ever. If you absolutely must use `mysql_real_escape_string`, do it directly. Do not do it through misdirection or a confusing layer of abstraction. – tadman Oct 30 '12 at 21:43
  • What I don't seem to understand here is, how is passing a string inside a PHP function to later on filter it via mysqli dangerous? It's basically like doing `mysqli_real_escape_string`, the only difference is that I have it inside a function, so that's what I would like to know. Thank you. It doesn't say `mysql_real_escape_string` though, just `mysqli_real_escape_string`. – Ignacio Belhot Colistro Oct 30 '12 at 21:48
  • I already have this clear, thank you for your advice. – Ignacio Belhot Colistro Oct 30 '12 at 22:54
  • 1
    As some people have pointed out, if that handle isn't valid then the function does not operate as intended. It's awful enough to be using that escape function, or any variation of it, directly. Adding an abstraction layer tries to clean up what *should* be ugly. It must be completely obvious where any escaping is done within an application, so the escape calls must be exposed. If you're using proper SQL placeholders, where this wouldn't be an issue. – tadman Oct 31 '12 at 14:24
  • @tadman Thank you for clearing up, the handle is valid, I've been using that for a long time and it works smooth. I agree with you though, It's just better to use parametrized queries than to escape it like this manually. I guess I'll start using PDO soon, the only problem with it is that I'll have to re-write a vast amount of the application again. But best for the better than for the worse I guess. – Ignacio Belhot Colistro Oct 31 '12 at 15:11
  • Any application written with PDO is automatically more readable and easier to audit because of it. You'll probably find more than a few "whoops, forgot to escape that" instances if you go through the tedious but essential step of switching to it. I hope "soon" means "on my next project" for you. `mysql_query` has no business being used in 2012. – tadman Oct 31 '12 at 16:18
0

If your ID it's an integer, just use

$postID = (int)$_POST['id'];

And of course, validate if $postID it's != zero after that line.

Juanma
  • 1,651
  • 1
  • 11
  • 15
  • This obviously does nothing to help any other variable types and is a pretty inconsistent way of handling all user submitted content. – Joel Mellon Oct 30 '12 at 21:18
  • If you think this is a form of security, you've been practicing some very bad habits. Don't do this. – tadman Oct 30 '12 at 21:41
0

Is it really necessary to use PDO/mysqli stuff?

No.

Is it sufficient to analyze your sql strings to be processed in mysql?

No.

Learning Material

Coding Security Controls

Don’t write your own security controls! Reinventing the wheel when it comes to developing security controls for every web application or web service leads to wasted time and massive security holes. The OWASP Enterprise Security API (ESAPI) Toolkits help software developers guard against security‐related design and implementation flaws - https://code.google.com/p/owasp-esapi-php/

Anthony Hatzopoulos
  • 10,437
  • 2
  • 40
  • 57
-2

Just use PDO and prepare your statements.

Klemen Tusar
  • 9,261
  • 4
  • 31
  • 28
  • While technically correct, there are much better answers here than this which only barely meets the minimum required to post. – tadman Oct 30 '12 at 21:41