MySQL Injection Problem

Question

I've been coding my website in PHP lately and I was pretty proud of myself for my good practices of sanitizing my input before I used it in a query. It was all going great until my friend said I need to sanitize my input. When I tried to explain to him that it was sanitized, he showed me that he had found everything in 'users' table in my database. I didn't know how, so I thought I would post what I was doing wrong that made my sanitizing not work. Here is the PHP code he was exploiting:

start_mysql(); // Starts the databases stuff, etc.

$id = mysql_real_escape_string($_GET['id']);
$game = mysql_query("SELECT * FROM `games` WHERE `id` = $id LIMIT 0, 1");

All he was doing was changing the id parameter, making him able to use SQL injection on my database. I thought mysql_real_escape_string escaped all characters like that, but apparently I was wrong. I did some tests with a normal string to see what would happen, and this is what it said

URL: /game.php?id=' OR '' = '

echo($_GET['id']); // This echo'd: \' OR \'\' = \'
echo(mysql_real_escape_string($_GET['id'])); // This echo'd: \\\' OR \\\'\\\' = \\\'

So, my simple question is, what am I doing wrong?

Why don't you ask your friend what request string he used? I'd be curious to know as well. — ben, Jun 16 '11 at 19:33

score 6 · Answer 1 · answered Jun 16 '11 at 19:35

6

You need to put the escaped string in single quotes:

WHERE `id` = '$id'

answered Jun 16 '11 at 19:35

SLaks

868,454
176
1,908
1,964

Surrounding user input with quotes does not protect against injection attacks, since users can supply their own closing quotes. – Ted Hopp Jun 16 '11 at 19:39

score 4 · Answer 2 · answered Jun 16 '11 at 19:36

Since id was an integer parameter and you did not surround it in single-quotes in your SQL, the value of $id is sent directly into your query. If you were expecting an integer id, then you should verify that the value of $_GET['id'] is a valid integer.

$id = intval($_GET['id']);

score 3 · Accepted Answer · edited May 23 '17 at 12:20

3

Matt,

mysql_real_escape_string() will only filter for certain characters, if you truly want to prevent injection attacks check out this other Stack Overflow article that suggests you use Prepared statements:

Prepared Statements

PHP Manual entry on Prepared statements

Edit: Also check out Slaks and Michael's postings about wrapping your variable in single quotes.

Good luck!

H

edited May 23 '17 at 12:20

Community

1
1

answered Jun 16 '11 at 19:40

hypervisor666

1,275
1
9
17

I wish I would have implemented this earlier. Guess I should start editing :P – Matt Jun 16 '11 at 19:51
@Matt, no worries bud, it took me a while to learn the "proper" way of filtering out injection attacks, I stumbled across that Stack Overflow article and was like "Holy Moly, I was doing it wrong!". But now you know, and "knowing is half the battle" :) – hypervisor666 Jun 16 '11 at 19:56
lol. Anyways, thanks for the response. This way seems a lot better looking also. – Matt Jun 16 '11 at 20:01

Ted Hopp · Answer 4 · 2011-06-16T19:43:15.137

1

~~You need to use the parameter binding api. The problem is in this piece of code:~~

WHERE `id` = $id

You are directly interpolating user input into your SQL statement. That's the open barn door for SQL injection attacks.

edited Jun 16 '11 at 19:43

answered Jun 16 '11 at 19:32

Ted Hopp

232,168
48
399
521

This is wrong. I missed that you were already escaping. Use the answer by Slaks. – Ted Hopp Jun 16 '11 at 19:42
Skipping the escape will open up security flaws. – Louis-Philippe Huberdeau Jun 16 '11 at 19:49
@Louis - I agree. What was wrong with my answer was that he was already escaping. Using `mysql_real_escape_string` like he was doing is a perfectly fine _alternative_ to using the parameter binding api. They shouldn't be used together. – Ted Hopp Jun 16 '11 at 19:56

score 1 · Answer 5 · answered Jun 16 '11 at 19:34

1

Cast ID. If it is a string it will cast as 0. $id = (int)$_GET['id'];

Also, MySQL support quotes around both string and numbers in the query.

$game = mysql_query("SELECT * FROM `games` WHERE `id` = '$id' LIMIT 0, 1");

answered Jun 16 '11 at 19:34

gpresland

1,690
2
20
31

score 0 · Answer 6 · edited Jun 01 '12 at 14:24

0

You can't prevent SQL injections using mysql_real_escape_string(). It is used for escaping special characters like single quotes ('), double quotes ("), etc.

To prevent SQL injections you have to use PDO statements and filter functions in PHP for sanitizing the user data.

edited Jun 01 '12 at 14:24

Maehler

6,111
1
41
46

answered Jun 01 '12 at 14:11

shanmugavel

199
1
3
13

score 0 · Answer 7 · answered Jun 16 '11 at 19:32

0

You're not using parameterized queries.

MDB2 allows this, though that library may be falling out of favor.

answered Jun 16 '11 at 19:32

Andrew

14,325
4
43
64

1

in PHP parameterized queries are known as "prepared statements". Link: [PHP pdo Prepared Statements](http://us3.php.net/manual/en/class.pdostatement.php) – hypervisor666 Jun 16 '11 at 19:41

score 0 · Answer 8 · answered Jun 16 '11 at 19:48

It's very likely that your configuration has magic_quote_gpc, an ancien attempt in PHP to make scripts secure magically. It proved to have multiple flaws and was since deprecated and was scheduled to be completely removed in 5.4 the last time I heard of it.

If you have access to your php.ini configuration, you should disable it. Otherwise, you can modify your script to take it into account and sanitize your input.

All of this is documented here: http://www.php.net/manual/en/security.magicquotes.disabling.php

Otherwise, there is nothing wrong with mysqli_real_escape_string().

MySQL Injection Problem

8 Answers8