Protection from SQL Injections

[esahp]

I have a signup form, and the data on it will get inserted into a MySQL database. Now as far as I know I've taken the proper steps in the following code to prevent SQL, javascript, and html source injections. Is there anything further I can do? Or is what I already have done it.

I have the signup form action go to another page, and the contents of said page are:
[code]
<?
  $firstname = mysql_escape_string(strip_tags($_POST['firstname']));
  $lastname = mysql_escape_string(strip_tags($_POST['lastname']));
  $email = mysql_escape_string(strip_tags($_POST['email']));
  $phonenumber = mysql_escape_string(strip_tags($_POST['phonenumber']));
  $homeaddress = mysql_escape_string(strip_tags($_POST['homeaddress']));
  $citystate = mysql_escape_string(strip_tags($_POST['citystate']));
  $country = mysql_escape_string(strip_tags($_POST['country']));
  $domainname = mysql_escape_string(strip_tags($_POST['domainname']));
  $username = mysql_escape_string(strip_tags($_POST['username']));
  $password1 = mysql_escape_string(strip_tags($_POST['password1']));
  $password2 = mysql_escape_string(strip_tags($_POST['password2']));
  $rules = mysql_escape_string(strip_tags($_POST['rules']));
  $legalinfo = mysql_escape_string(strip_tags($_POST['legalinfo']));
  $age = mysql_escape_string(strip_tags($_POST['age']));
  $sitedetails = mysql_escape_string(strip_tags($_POST['sitedetails']));
  $aboutus = mysql_escape_string(strip_tags($_POST['aboutus']));
  if ($firstname == "") { $errors .= "First Name field was left blank.<br />"; }
  if ($lastname == "") { $errors .= "Last Name field was left blank.<br />"; }
  if ($email == "") { $errors .= "Email Address field was left blank.<br />"; }
  if ($phonenumber == "") { $errors .= "Phone Number field was left blank.<br />"; }
  if ($homeaddress == "") { $errors .= "Home Address Field was left blank.<br />"; }
  if ($citystate == "") { $errors .= "City&State field was left blank.<br />"; }
  if ($country == "") { $errors .= "Country field was left blank.<br />"; }
  if ($domainname == "") { $errors .= "Your Domain field was left blank.<br />"; }
  if ($username == "") { $errors .= "Desired Username field was left blank.<br />"; }
  if (($password1 == "") || ($password2 == "") || ($password1 != $password2)) { $errors .= "Password fields were left blank or do not match.<br />"; }
  if ($rules == "") { $errors .= "You didn't agree to the rules.<br />"; }
  if ($legalinfo == "") { $errors .= "You didnt agree to the legal information.<br />"; }
  if ($age == "") { $errors .= "You didnt state you were over the age of 18.<br />"; }
  if ($sitedetails == "") { $errors .= "Site Details field was left blank.<br />"; }
  if ($aboutus == "") { $errors .= "About Us field was left blank.<br />"; }
  if (isset($errors)) {
    echo $errors;
  }
  else {
    // SQL Query stuff here
    echo "Works!";
  }
?>
[/code]

[oracle259]

You should also check the magic_quotes_gpc state (ie on or off) you can try something like this also you can tie ur mysql_real_escape_string tests into the function

[code]

function makesafe($string) {

if (!isset($_REQUEST['$string']) || empty($_REQUEST['$string'])) {
die ("Unauthorized action"); }

if (!get_magic_quotes_gpc()) {
  $string = addslashes($_POST['$string']);
} else {
  $string = $_POST['$string'];
}
$string = mysql_real_escape_string($string);
return $string

}

[/code]

Remember though, if you are going to use empty $string can't be 0 or it will return empty.

[esahp]

I was told I didn't need addslashes(); if I had both mysql_escape_string(); and strip_tags();

[oracle259]

Im still a newbie so may be but doesnt hurt to do it

[extrovertive]

if (get_magic_quotes_gpc()) {
  $string = stripslashes($_POST['$string']);
} else {
  $string = $_POST['$string'];
}
$string = mysql_real_escape_string($string);
return $string

[esahp]

That still didn't answer my question.

First post: "Is there anything further I can do? Or is what I already have done it."

Also, quoting my second post "I was told I didn't need addslashes(); if I had both mysql_escape_string(); and strip_tags();" is this true that I don't need addslashes(); if I have both of the following, or do I still need to include it somewhere?

And whats the deal with magic_quotes_gpc?

[kenrbnsn]

If you use mysql_real_escape_string() you should not use addslashes().

Ken

[esahp]

Thankyou for giving me a straight answer.

[bob_the _builder]

Hi,

Save yourself some repetitive typing and use a function:

[code=php:0]function ValidateInput($value) {

if (!get_magic_quotes_gpc()) {
  $value = mysql_real_escape_string($value);
}
$value = trim(strip_tags($value));
return $value;
}

$firstname = ValidateInput($_POST['firstname']);
$lastname = ValidateInput($_POST['lastname']);[/code]


Bob

[esahp]

Actually, with the power of ctrl+v it was quite easy :P

I noticed you have mysql_real_escape_string(); whereas I have mysql_escape_string(). From the looks of the php manual mysql_real_escape_string(); is better. Is that true?

[Jenk]

Don't need to use strip_tags or trim.

[code]<?php

function magic_clean ($string)
{
    if (get_magic_quotes_gpc()) $string = mysql_real_escape_string($string);

    return $string;
}

?>[/code]

Only escape for what you need to escape for, no point using strip_tags if all you are doing is inserting to the DB... you may actually want to allow users to use HTML tags.

trim is just unecessary.

You also need to refine your variable checking:

[code]<?php

if (!empty($_GET['variable'])) $variable = magic_clean($_GET['variable']);

?>[/code]

[wildteen88]

You can change the whole of the following:
[code]$firstname = mysql_escape_string(strip_tags($_POST['firstname']));
  $lastname = mysql_escape_string(strip_tags($_POST['lastname']));
  $email = mysql_escape_string(strip_tags($_POST['email']));
  $phonenumber = mysql_escape_string(strip_tags($_POST['phonenumber']));
  $homeaddress = mysql_escape_string(strip_tags($_POST['homeaddress']));
  $citystate = mysql_escape_string(strip_tags($_POST['citystate']));
  $country = mysql_escape_string(strip_tags($_POST['country']));
  $domainname = mysql_escape_string(strip_tags($_POST['domainname']));
  $username = mysql_escape_string(strip_tags($_POST['username']));
  $password1 = mysql_escape_string(strip_tags($_POST['password1']));
  $password2 = mysql_escape_string(strip_tags($_POST['password2']));
  $rules = mysql_escape_string(strip_tags($_POST['rules']));
  $legalinfo = mysql_escape_string(strip_tags($_POST['legalinfo']));
  $age = mysql_escape_string(strip_tags($_POST['age']));
  $sitedetails = mysql_escape_string(strip_tags($_POST['sitedetails']));
  $aboutus = mysql_escape_string(strip_tags($_POST['aboutus']));
[/code]

Into just the following few lines:
[code=php:0]foreach($_POST as $field => $value)
{
    if(isset($_POST[$field]) && !empty($_POST[$field]))
    {
        ${$field} = mysql_real_escape_string(strip_tags($value));
    }
}[/code]

[bob_the _builder]

Hi,

Personally to lower sql injection risk I would introduce bbcode and then striptags, trim just helps to keep everything tidy.


Bob