DB Input Security AND DB not request flawed

Jane_Smith

Hi,

I am trying to security protect my database. I have javascript validation on the html form, but I am aware that this can be disabled so want to check the insert page is suitably protected.

Firstly: Am I right in think real_escape_string is sufficient protection, or do I need to validate and sanitize as well?

Secondly: Have I applied it correctly below, I think I can write it as per below to check multiple fields, but I'm not certain I interpretted this correctly.

Thirdly: I am getting mysql error on this page, I can not work out what I am doing wrong, any pointers?

Any help would be much appreciated.

<html>
<head>

</head>
<body>
<?php
function check_input($value)
{
if (get_magic_quotes_gpc())
   {
   $value=stripslashes($value);
   }

if (!is_numeric($value))
   {
   $value = "'" . mysql_real_escape_string($value) . "'";
   }
return $value;
}

   $con = mysql_connect("localhost","user","password");
   if (!$con)
      {
      die('Could not connect: ' . mysql_error());
      }

   mysql_select_db("my_db", $con);

   $sql="INSERT INTO table(fname, lname, email, cemail, pword, cpword, sex, DOBMonth, DOBDay, DOBYear, natcountry, rcountry, postcode, industry, other, date)
      VALUES
   ('$_POST[fname]','$_POST[lname]','$_POST[email]','$_POST[cemail]','$_POST[pword]','$_POST[cpword]','$_POST[sex]','$_POST[DoBMonth]','$_POST[DOBDay]','$_POST[DoBYear]','$_POST[sex]','$_POST[ncountry]','$_POST[postcode]','$_POST[industry]','$_POST[other]','$_GET[CURDATE]')";

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

   mysql_close($con);

?>

</body>
</html>

NogDog

I did not review your code with a fine tooth comb, but applying mysql_real_escape_string() to all external inputs should be sufficient to address SQL injection issues, which should protect the actual database. Other things for possible security considerations would be the actual content, and whether you want to reject or filter out HTML tags or anything else (such as might be used for cross-site scripting, spam links, etc.

PS: If you use [noparse]

 tags instead of [code][/noparse] tags around your PHP code here, you get context-sensitive text coloring, which is both useful and pretty. :)

bradgrafelman

NogDog wrote:
applying mysql_real_escape_string() to all external inputs should be sufficient to address SQL injection issues

Actually, it isn't sufficient in the case where the input isn't string data (hence the function's name). In that case, using mysql_real_escape_string() is actually incorrect (e.g. "3 OR 1=1" won't be changed at all by mysql_real_escape_string(), yet it's obviously not the numeric input you were expecting).

NogDog

bradgrafelman;10963329 wrote:
Actually, it isn't sufficient in the case where the input isn't string data (hence the function's name). In that case, using mysql_real_escape_string() is actually incorrect (e.g. "3 OR 1=1" won't be changed at all by mysql_real_escape_string(), yet it's obviously not the numeric input you were expecting).

OK, nit-picker, let me rephrase my response, then: "applying mysql_real_escape_string() and adding quotes as needed to all external inputs as you are with your check_input() function should be sufficient to address SQL injection issues."

Happy?