Jump to content


Photo

Protection from SQL Injections


  • Please log in to reply
12 replies to this topic

#1 esahp

esahp
  • Members
  • PipPip
  • Member
  • 28 posts
  • LocationUSA

Posted 28 September 2006 - 02:59 AM

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:
<?
  $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!";
  }
?>

To obtain peace and quiet, get a Phoneless Cord.


#2 oracle259

oracle259
  • Members
  • PipPipPip
  • Advanced Member
  • 119 posts

Posted 28 September 2006 - 03:48 AM

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


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

}


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

#3 esahp

esahp
  • Members
  • PipPip
  • Member
  • 28 posts
  • LocationUSA

Posted 28 September 2006 - 04:07 AM

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

To obtain peace and quiet, get a Phoneless Cord.


#4 oracle259

oracle259
  • Members
  • PipPipPip
  • Advanced Member
  • 119 posts

Posted 28 September 2006 - 04:09 AM

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

#5 extrovertive

extrovertive
  • Members
  • PipPipPip
  • Advanced Member
  • 235 posts

Posted 28 September 2006 - 04:12 AM

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


#6 esahp

esahp
  • Members
  • PipPip
  • Member
  • 28 posts
  • LocationUSA

Posted 28 September 2006 - 04:17 AM

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?

To obtain peace and quiet, get a Phoneless Cord.


#7 kenrbnsn

kenrbnsn
  • Staff Alumni
  • Advanced Member
  • 8,235 posts
  • LocationHillsborough, NJ, USA

Posted 28 September 2006 - 04:57 AM

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

Ken

#8 esahp

esahp
  • Members
  • PipPip
  • Member
  • 28 posts
  • LocationUSA

Posted 28 September 2006 - 05:15 AM

Thankyou for giving me a straight answer.

To obtain peace and quiet, get a Phoneless Cord.


#9 bob_the _builder

bob_the _builder
  • Members
  • PipPipPip
  • Advanced Member
  • 207 posts

Posted 28 September 2006 - 06:04 AM

Hi,

Save yourself some repetitive typing and use a function:

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']);


Bob


#10 esahp

esahp
  • Members
  • PipPip
  • Member
  • 28 posts
  • LocationUSA

Posted 28 September 2006 - 02:34 PM

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?

To obtain peace and quiet, get a Phoneless Cord.


#11 Jenk

Jenk
  • Members
  • PipPipPip
  • Advanced Member
  • 778 posts

Posted 28 September 2006 - 02:45 PM

Don't need to use strip_tags or trim.

<?php

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

    return $string;
}

?>

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:

<?php

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

?>


#12 wildteen88

wildteen88
  • Staff Alumni
  • Advanced Member
  • 10,482 posts
  • LocationUK, Bournemouth

Posted 28 September 2006 - 02:58 PM

You can change the whole of the following:
$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']));

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


#13 bob_the _builder

bob_the _builder
  • Members
  • PipPipPip
  • Advanced Member
  • 207 posts

Posted 28 September 2006 - 08:59 PM

Hi,

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


Bob




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users