SQL Update code issue/PHP injection

Question

I am having an issue with my SQL Update script. It prints "Motto Changed" but doesn't update the row. My code is all correct according to many tutorials. Please Help

$sql="UPDATE loadout SET motto='".$_POST['motto']."'  WHERE steamid='".$steamid."'";

UPDATE AGAIN:

<?php
   require "../requires/php/steam.php";
 $dbhost  = '**';
 $dbname  = 'battlefield';
 $dbuser  = 'battlefield';
 $dbpass  = '**'; 

$con = mysql_connect($dbhost, $dbuser, $dbpass);

$authserver = bcsub( SteamID(), '76561197960265728' ) & 1;
    $authid = ( bcsub( SteamID(), '76561197960265728' ) - $authserver ) / 2;
$steamid = mysql_real_escape_string("STEAM_0:$authserver:$authid");




$motto = mysql_real_escape_string($_POST['motto']);

mysql_select_db($dbname, $con);


$sql="UPDATE loadout SET motto='{$motto}'  WHERE steamid='{$steamid}'";


if (!mysql_query($sql, $con))
{
   die('Error: ' . mysql_error());
}
echo "Motto Changed";

if (!mysql_query($sql, $con))
{
die('Error: ' . mysql_error());
}
 $n = mysql_affected_rows();
echo"Motto changed on {$n} row(s)";

mysql_close($con)
?>

Your code is vulnerable to SQL-injection, just so you know. And, posting how you send the query and more can be helpful. — Max, Jul 10 '14 at 17:26
Is the change ever committed? Can you print the resulting sql statement string? — Vulcronos, Jul 10 '14 at 17:29
Is $steamid being set correctly? Does that id actually exist in your db? — WillardSolutions, Jul 10 '14 at 17:35
echo the $steamid before update query to check it is set and check whether there is an item with that id in your database? — Burak, Jul 10 '14 at 17:36
Yes, steamid is a column that contains the users SteamID, they have to be logged in to access this page. — user3826505, Jul 10 '14 at 17:36
Why are you escaping $_POST['motto'] to $motto and then using $_POST['motto'] in the SQL?? — Arth, Jul 10 '14 at 17:37

score 0 · Accepted Answer · edited May 23 '17 at 10:32

Never interpolate $_POST variables directly into SQL strings. You can't trust $_POST variables, they may easily contain characters that modify your SQL syntax, and that's what causes SQL injection vulnerabilties.

The weird thing is that you create an escaped version as $motto and then you never use it (as per comment from @Arth).

Always escape strings that you interpolate into SQL, even if you think they are "safe." For example, your $steamid contains only literal text that you control, plus a couple of integers. That should be safe, but what if some other developer changes the format of a steamid next year? If you escape it, you can't go wrong.

$steamid = mysql_real_escape_string("STEAM_0:$authserver:$authid");

$motto = mysql_real_escape_string($_POST['motto']);

$sql="UPDATE loadout SET motto='{$motto}'  WHERE steamid='{$steamid}'";

Of course, the best practice is to use query parameters. You are using PHP's deprecated mysql extension, which doesn't support query parameters. But I understand if you're not ready to rewrite a lot of code to switch to PDO. When you are, follow examples in How can I prevent SQL-injection in PHP?

Another issue: if you want to know if the UPDATE affected rows, don't assume it did just because the UPDATE didn't return an error. It's not an error if your condition in your WHERE clause simply matched zero rows. It's also not an error if the UPDATE matched a row, but the motto already contained the string you tried to set.

After the UPDATE, check the number of affected rows:

if (!mysql_query($sql, $con))
{
    die('Error: ' . mysql_error());
}
$n = mysql_affected_rows();
echo "Motto changed on {$n} row(s)";

The changes to told me to make still don't work? i will update in the question. — user3826505, Jul 10 '14 at 18:04
I didn't mean for you to execute the update twice before you request the number of affected rows. The second update will report 0 rows changed, because when UPDATE changes values to the values the row already contained, it doesn't count as an "affected row." — Bill Karwin, Jul 10 '14 at 18:10

SQL Update code issue/PHP injection

1 Answers1