How i can make my code secure from SQL Injection

Question

I know i am not secure when i am using this code so anything i can add in my code?

I have tried my self sql injection they are somewhere working but not much as i dont have much knowledge about sql injection. but as hacker are more smart so they can really hack my website.

Url looks like this :

http://example.com/profile.php?userID=1

php

$userID = $_GET['userID'];
$userID = mysql_real_escape_string($userID);
$CheckQuery = mysql_query("SELECT * FROM tbl_user WHERE id='$userID'");

$CheckNumber = mysql_num_rows($CheckQuery);
if ($CheckNumber !== 1)
{
    header("Location: tos.php");
}

I tried:

http://example.com/profile.php?userID=1'

which hide many things on site.

when i tried

http://example.com/profile.php?userID=1' UNION SELECT * FROM tbl_user; with havij it was hacked

Thanks :|

What if you try `http://example.com/profile.php?userID=0` - will it "hide many things" too? — Your Common Sense, May 22 '13 at 08:13

score 0 · Answer 1 · answered May 22 '13 at 07:44

use mysqli::prepare or at least sprintf

mysql_query(sprintf("SELECT * FROM tbl_user WHERE id='%d'", $userID);

$db = new mysqli(<database connection info here>);

$stmt = $db->prepare("SELECT * FROM tbl_user WHERE id='?'");
$stmt->bind_param("id", $userID);
$stmt->execute();
$stmt->close();

score 0 · Answer 2 · answered May 22 '13 at 07:48

0

Dont use mysql_* functionality at all. Use PDO or mysqli.

http://php.net/PDO http://php.net/mysqli

PDO will escape your data for you.

But for your current code:

$userID = $_GET['userID'];
$userID = mysql_real_escape_string($userID);

if(ctype_digit($userID))
{
    $CheckQuery = mysql_query("SELECT * FROM tbl_user WHERE id='$userID'");

    $CheckNumber = mysql_num_rows($CheckQuery);
    if ($CheckNumber !== 1)
    {
        header("Location: tos.php");
    }
} else {
    // THE USER ID IS NOT ALL NUMBERS, CREATE AN ERROR
}

answered May 22 '13 at 07:48

Phil Cross

9,017
12
50
84

Its a old website and if i try to change codes it will take more then month :( – deerox May 22 '13 at 07:51
There's no substitute for good security. Unfortunately, if your database gets hacked because the security used is poor, your company / your person is likely to get into trouble for it. – Phil Cross May 22 '13 at 07:52
yes so true. its an online game – deerox May 22 '13 at 07:55
Parse error: syntax error, unexpected T_STRING – deerox May 22 '13 at 08:01
Why the downvote? There isn't a parse error in the code mentioned above. I only added 1 meaningful line of code, which was to ensure the userID value is a number. `if(ctype_digit($userID))...` – Phil Cross May 22 '13 at 08:10

Your Common Sense · Answer 3 · 2013-05-22T08:39:42.350

I know i am not secure when i am using this code

This statement is wrong.
As a matter of fact, this very code is pretty secure.

And none of the codes you provided below would do any harm. Why do you think it is not secure?

This way is not recommended, yes. And the way you are using to format your queries may lead to injection for some other query. But the present code is perfectly secure.

As long as you are enclosing every variable in quotes and escape special chars in it - it is safe to be put into query.
Only if you omit one these two rules (i.e. escape but don't quote or quote but don't escape) - you are in sure danger. But as long as you're following both, you're safe.

The only reason for "hacking" I can guess of is a single quote used in HTML context. In some circumstances it can "hide many things on the page". But for the SQL, with the code you posted here, it's harmless

Look, out of this link

http://example.com/profile.php?userID=1'

your code will produce such a query

SELECT * FROM tbl_user WHERE id='1\''

which is quite legit for mysql and will even return a record for id=1, as it will cast 1' to 1 and find the record. This is why there is no redirect to tos.php.

So, the problem is somewhere else.

either there is a code that does not follow the rules I posted above
or this problem is unrelated to SQL at all - so, you are barking wrong tree and thus still keep whatever vulnerability open

Most likely you have to echo your values out

atleast it should take them to tos.php when they use : http://example.com/profile.php?userID=1' — deerox, May 22 '13 at 08:10

score -1 · Answer 4 · answered May 22 '13 at 07:43

-1

u can try type casting the value

<?php


$CheckQuery = mysql_query("SELECT * FROM tbl_user WHERE id='".(int)$userID."'");

?>

answered May 22 '13 at 07:43

sAnS

1,169
1
7
10

No, this wont work this way! This way `(int)` will just be part of the query string. What you could try is `$userID = (int)$userID;` before doing the query string line. – nl-x May 22 '13 at 07:46
check the query now.. – sAnS May 22 '13 at 07:48

How i can make my code secure from SQL Injection

4 Answers4