Why wont this mysql update script work?

Question

I have been beating my head against a wall for a few hours now trying to get this to update my DB.

<?
//edit_item_data.php
$con=mysqli_connect("localhost","root","","Inventory");
// Check connection
if (mysqli_connect_errno())
  {
  echo "Failed to connect to MySQL: " . mysqli_connect_error();
  }
  $sql= "UPDATE Item 
          SET Catagory = '$_POST[Catagory]',
          Cost = '$_POST[Cost]',
          Condition = '$_POST[Condition]',
          PurchaseLot_PurchaseLotID = '$_POST[PurchaseLot]',
          Location = '$_POST[Location]',
          Desc = '$_POST[Desc]',
          Notes = '$_POST[Notes]'
          WHERE 
          ItemID = '$_POST[id]'";

if (!mysqli_query($con,$sql))
  {
  die('Error: ' . mysqli_error($con));
  }
echo "1 record added";

mysqli_close($con);
?>
<script type='text/javascript'>
 settimeout('self.close()',5000);
</script>

this is the error I'm getting

Error: You have an error in your SQL syntax; check the manual that corresponds to 
your MySQL server version for the right syntax to use near 'Condition = New,     
PurchaseLot_PurchaseLotID = 1, Location = e' at line 4

I'm running mysql 5.6 and php 5.5. I'm sure its something dumb but I can't for the life of me see what it is.

Your SQL statement is invalid because you have several string variables placed unquoted directly into it, which is invalid. What you have is highly [vulnerable to tampering via SQL injection](http://en.wikipedia.org/wiki/Sql_injection). It is a good time to take a step back and read the [MySQLi documentation on prepared statements](http://php.net/manual/en/mysqli.prepare.php) to alleviate both your quoting problem and the large security problem. — Michael Berkowski, Dec 14 '13 at 23:28

score 1 · Answer 1 · answered Dec 14 '13 at 23:29

1

Well you are hilariously vulnerable to SQL injection doing what you are doing, but the problem is that you aren't enclosing your variables in quotes, e.g:

SET Catagory = '$_POST[Catagory]',
-- etc

Use mysqli_real_escape_string to escape your variables before you put them into your SQL, like this:

SET Catagory = '" . mysqli_real_escape_string($_POST['Catagory'], $con) . "',

answered Dec 14 '13 at 23:29

scrowler

24,273
9
60
92

I have try this but it still is gives me the same error message – DcHerrera Dec 14 '13 at 23:32
this will be made more secure once i understand prepared staments a little better. But this will also exist in a vpn environment only. It will never see the open web. – DcHerrera Dec 14 '13 at 23:33
Fair enough - you've put quotes around all of your variables? – scrowler Dec 14 '13 at 23:38
I've updated the code above to reflect the changes I made. Its still giving me the same error. – DcHerrera Dec 14 '13 at 23:45
Dear Lord.(Pardon if that offends) It was back ticks..````` – DcHerrera Dec 14 '13 at 23:46

score 0 · Answer 2 · edited Feb 08 '17 at 14:07

0

xkcd

You want something like this:

$sql = "UPDATE `Item` SET
   `Catagory` = '".mysqli_real_escape_string($_POST['Catagory'],$con)."',
   `Cost` = '".mysqli_real_escape_string($_POST['Cost'],$con)."',
   ........
   WHERE `ItemID` = ".intval($_POST['id']);

Side-note, it's spelled "category".

EDIT: If you, like me, can't be arsed to type out such a long function name...

$e = function($str) use ($con) {
    return mysqli_real_escape_string($str,$con);
};

Then:

... `Catagory` = '".$e($_POST['Catagory'])."' ...

edited Feb 08 '17 at 14:07

Community

1
1

answered Dec 14 '13 at 23:30

Niet the Dark Absol

320,036
81
464
592

mysql_* is deprecated, -1 for suggesting - **edit** nice quick edit there, I'll take it back – scrowler Dec 14 '13 at 23:30
Psh, so I missed the "i". Sue me. – Niet the Dark Absol Dec 14 '13 at 23:30
and the connection resource – scrowler Dec 14 '13 at 23:31
Also, prepared statements are more robust than `*_real_escape_string` – Kermit Dec 15 '13 at 00:01
@FreshPrinceOfSO Unless you need variable column names. – Niet the Dark Absol Dec 15 '13 at 00:03
@FreshPrinceOfSO *shrugs* I still use `mysql_query`, so I can't really argue. The vulnerabilities come from improper use, not an innate problem. – Niet the Dark Absol Dec 15 '13 at 00:12
1

@NiettheDarkAbsol Have fun with that. I'll leave you with [this](http://stackoverflow.com/questions/732561/why-is-using-a-mysql-prepared-statement-more-secure-than-using-the-common-escape). – Kermit Dec 15 '13 at 17:21

score 0 · Accepted Answer · answered Jan 08 '14 at 14:51

0

The real issue was lack of grave accents

        SET `Catagory` = '$_POST[Catagory]',

answered Jan 08 '14 at 14:51

DcHerrera

80
1
11

Why wont this mysql update script work?

3 Answers3