CODEWITHSUNDEEP
phpmysqlsqlsql-injection
MstrQKN
MstrQKN

Reputation: 844

Am I using mysql_real_escape_string right?

Is this the right way to use mysql_real_escape_string? I was using $GET but a friend told me to make it safer with real_escape_string:

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

$result = mysql_query("SELECT * 
                         FROM products 
                        WHERE id = $id") or die("err0r");

if(!$result) mysql_real_escape_string($id); {

Upvotes: 1

Views: 5651

Answers (5)

robx
robx

Reputation: 3123

you want to escape it before you stick it in a query (Before it interacts with DB so you don't get injections).

// check if your $_GET is not empty otherwise you 
// will run into "undefined variable"
if(!empty($_GET['id'])){
    $id = intval($_GET['id']);

    // to simplify you can escape here, 
    // or to be a bit more complex, you can escape in the query line.
    $id = mysql_real_escape_string($id); 

    $result = mysql_query("SELECT * 
                         FROM products 
                        WHERE id = '$id'") or die("err0r");
}
else
    print 'No ID';

Upvotes: 1

lonesomeday
lonesomeday

Reputation: 238035

No. That is entirely wrong, and I can't quite understand what you're intending the call to do.

The purpose of mysql_real_escape_string is to avoid SQL injection, which is one of the biggest security risks in a website. It stops your users giving input that manipulates the SQL in evil ways. For instance:

$sql = "SELECT FROM users WHERE username = '" . $_GET['username'] . "'";

If I put lonesomeday' or 'a' = 'a into $_GET['username'], your query becomes

SELECT FROM users WHERE username = 'lonesomeday' or 'a' = 'a'

and obviously arbitrary SQL could then be executed. mysql_real_escape_string escapes unsafe characters (such as ' in that example), so that they can't be used in this way. 

$sql = "SELECT FROM users WHERE username = '" . mysql_real_escape_string($_GET['username']) . "'";
// SELECT FROM users WHERE username = 'lonesomeday\' or \'a\' = \'a'

The quotes are now escaped. so the query can't be manipulated into doing evil things.

With all that said, in this case, intval does all you need. It also ensures that nothing that is not an integer can be in $id, so your code is safe here from SQL injection.

Upvotes: 2

OMG Ponies
OMG Ponies

Reputation: 332721

Use:

$query = sprintf("SELECT * 
                    FROM products 
                   WHERE id = %d",
                  intval($_GET['id']));

$result = mysql_query($query) or die("err0r");

You use mysql_real_escape_string before the value is used in the query, otherwise you're not handling the SQL injection attack.

Upvotes: 1

Igor
Igor

Reputation: 2659

NO, you need to escape before quering

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

$result = mysql_query("SELECT * 
                         FROM products 
                        WHERE id = '" . mysql_real_escape_string($id) . "'") or die("err0r");

if(!$result) {
}

Upvotes: 1

jeroen
jeroen

Reputation: 91762

No, you normally use mysql_real_escape_string to prepare variables for use in a query, but in your case:

  1. you already use intval;
  2. you use it in the wrong place.

You don't need it in your example.

Upvotes: 6

Related Questions