0

I'm learning PHP and SQL, and for this page I am trying to practice preventing SQL Injection. Right now I'm just trying it on two variables. ac1 and ac2. I get an mysql() die error when I submit. What could be wrong?

<?php
$host="localhost"; // Host name
$username="******"; // Mysql username
$password="******"; // Mysql password
$db_name="******_practice"; // Database name
$tbl_name="administration"; // Table name

// Connect to server and select databse.
$dbc = mysql_connect("$host", "$username", "$password")or die("cannot connect");
mysql_select_db("$db_name")or die("cannot select DB");

$ac1=$_POST['ac1'];
$ac2=$_POST['ac2'];
$fan=$_POST['fan'];
$na=$_POST['na'];
$dh=$_POST['dh'];

$tolerance1=$_POST['tolerance1'];
$temptime1=$_POST['temptime1'];
$tolerance2=$_POST['tolernce2'];
$temptime2=$_POST['temptime2'];
$tolerance3=$_POST['tolerance3'];
$temptime3=$_POST['temptime3'];
$tolerance4=$_POST['tolerance4'];
$temptime4=$_POST['temptime4'];
$tolerance5=$_POST['tolerance5'];
$temptime5=$_POST['temptime5'];

$humidtolerance1=$_POST['humidtolerance1'];
$humidtime1=$_POST['humidtime1'];
$humidtolerance2=$_POST['humidtolerance2'];
$humidtime2=$_POST['humidtime2'];
$humidtolerance3=$_POST['humidtolerance3'];
$humidtime3=$_POST['humidtime3'];
$humidtolerance4=$_POST['humidtolerance4'];
$humidtime4=$_POST['humidtime4'];
$humidtolerance5=$_POST['humidtolerance5'];
$humidtime5=$_POST['humidtime5'];
// To prevent MySQL injection (a form of internet hacking)
$ac1 = stripslashes($ac1);
$ac2 = stripslashes($ac2);
$ac1 = mysql_real_escape_string($ac1);
$ac2 = mysql_real_escape_string($ac2);


$custnum = 0;
$sql="UPDATE {$tbl_name} SET ac1 = '{$ac1}', ac2 = '{$ac2}', fan = '{$fan}', na = '{$na}', da = '{$dh}', tolerance1 = '{$tolerance1}', temptime1 = '{$temptime1}',tolerance2 = '{$tolerance2}', temptime2 = '{$temptime2}',tolerance3 = '{$tolerance3}', temptime3 = '{$temptime3}',tolerance4 = '{$tolerance4}', temptime4 = '{$temptime4}',tolerance5 = '{$tolerance5}', temptime5 = '{$temptime5}', humidtolerance1 = '{$humidtolerance1}', humidtime1 = '{$humidtime1}',humidtolerance2 = '{$humidtolerance2}', humidtime2 = '{$humidtime2}',humidtolerance3 = '{$humidtolerance3}', humidtime3 = '{$humidtime3}',humidtolerance4 = '{$humidtolerance4}', humidtime4 = '{$humidtime4}',humidtolerance5 = '{$humidtolerance5}', humidtime5 = '{$humidtime5}' WHERE custnum = '{$custnum}'";
  $result = mysql_query($sql)
    or die('Error querying database.');


//Send them back to the page they were at/
header("location:index.php");
?>
Daniel USAF
  • 29
  • 1
  • 4
  • 6
  • You should try echoing the contents of 'mysql_error()' somewhere within the 'die' message for your own benefit. It'll make it much easier to debug the specific problems. – Gian Mar 08 '12 at 18:07
  • 1
    It looks like there are quite a few variables that are not being sanitized using `mysql_real_escape_string` in your query. All the variables in your query that come from `$_POST` should be sanitized the same way `$ac1` and `$ac2` are. As for figuring out the error, try adding `mysql_error()` to your `die` output. Do make sure to remove this when the code goes into production. You don't want to output information about your database to end users. – Travesty3 Mar 08 '12 at 18:08
  • Try echoing `mysql_error()` to tell you what the error is, but like the other answers so far, I'd suggest looking at PDO. – AndrewR Mar 08 '12 at 18:09
  • @Travesty3 Do note that the OP said he was only trying `mysql_real_escape_string` out on `ac1` and `ac2`, for now. – summea Mar 08 '12 at 18:15
  • @summea: Ah, I didn't see that. – Travesty3 Mar 08 '12 at 18:16

3 Answers3

2

If you wanted to be a little more sure that SQL injection would be prevented, you may want to consider using PDO prepared statements (provided you are using PHP 5.1 or higher,) instead of using mysql_real_escape_string (the old way.)

If that isn't an option:

  • Are you sure that there is a custnum of 0 in your database? Often, database indexes tend to start at 1. If that's the case, it may be one reason why the query is failing (nothing to update.)
summea
  • 7,390
  • 4
  • 32
  • 48
  • What harm do the brackets cause? – Travesty3 Mar 08 '12 at 18:10
  • @Travesty3 I don't think they are needed here. I use them when I am accessing a multidimensional array within a double-quoted string... otherwise, they don't seem necessary. – summea Mar 08 '12 at 18:11
  • Maybe not necessary, but I don't think it's a bad idea. I use them everywhere that a PHP variable is inserted into a string. Makes it stand out easier in my editor and it's more consistent. – Travesty3 Mar 08 '12 at 18:15
  • @Travesty3 Not a bad idea for editors; I guess I'm just too used to old school stuff. :P – summea Mar 08 '12 at 18:18
0

You should try using mysqli or even better, use PDO and prepared statements. Calling mysql_query directly is usually not the best approach.

http://php.net/manual/en/book.pdo.php

Sam Heuck
  • 585
  • 5
  • 15
0

Must be quotes in your case:

$sql="UPDATE {$tbl_name} SET `ac1` = '{$ac1}', `ac2` = '{$ac2}', `fan` = '{$fan}', `na` = '{$na}', `da` = '{$dh}', `tolerance1` = '{$tolerance1}', `temptime1` = '{$temptime1}',`tolerance2` = '{$tolerance2}', `temptime2` = '{$temptime2}',`tolerance3` = '{$tolerance3}', `temptime3` = '{$temptime3}',`tolerance4` = '{$tolerance4}', `temptime4` = '{$temptime4}',`tolerance5` = '{$tolerance5}', `temptime5` = '{$temptime5}', `humidtolerance1` = '{$humidtolerance1}', `humidtime1` = '{$humidtime1}',`humidtolerance2` = '{$humidtolerance2}', `humidtime2` = '{$humidtime2}',`humidtolerance3` = '{$humidtolerance3}', `humidtime3` = '{$humidtime3}',`humidtolerance4` = '{$humidtolerance4}', `humidtime4` = '{$humidtime4}',`humidtolerance5` = '{$humidtolerance5}', `humidtime5` = '{$humidtime5}' WHERE `custnum` = '{$custnum}'";

I.e. you should put those field names in the backtick quotes since you put your values in single ones. Nevertheless, let me give you a piece of advice: in production mode you better use PDO, as advised earlier. Good luck

ᴍᴇʜᴏᴠ
  • 4,804
  • 4
  • 44
  • 57
  • Using single quotes for values doesn't necessitate backticks around field names. Backticks are more for [allowing the use of alternative characters](http://stackoverflow.com/a/261476/259457) and reserved words. – Travesty3 Mar 08 '12 at 18:26