php forms

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

php forms

Post by rogie »

Hi All

I need some help!!!!!!

I have always used Matts formmail to set up forms on my web site (I know I know)

I have now changed to php for processing. I am using the following code but I don't know enough to know if it is secure. I am new to php

Code: Select all

 
 
<?php
$recipient = "my email here"; 
 
$error = ""; 
 
$name = $_POST['name'];
$email = $_POST['email'];
$subject = "Enquiry"; 
$phone = $_POST['phone'];
$country = $_POST['country'];
$adults = $_POST['adults'];
$children = $_POST['children'];
$age = $_POST['age'];
$arrival_day = $_POST['arrival_day'];
$arrival_month = $_POST['arrival_month'];
$arrival_year = $_POST['arrival_year'];
$departure_day = $_POST['departure_day'];
$departure_month = $_POST['departure_month'];
$departure_year = $_POST['departure_year'];
$comments = $_POST['comments'];
$verification = $_POST['verification'];
 
$message = 
"Name: " . $name . "\n E-mail: " . $email . "\n Telephone: " . $phone . "\n Country: " . $country . "\n\n Number of Adults: " . $adults . "\n Number of Children: " . $children . "\n Age of Children: " . $age . "\n\n Date of Arrival: " . $arrival_day . "," . $arrival_month . "," . $arrival_year . "\n Date of Departure: " . $departure_day . "," . $departure_month . "," . $departure_year . "\n\n Comments: " . $comments . "\n";
 
 
$emailPattern = '/^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$/';
 
if(!preg_match($emailPattern, $email)) 
{
$error = "Incorrect Email. ";
}
if(md5($verification) != $_COOKIE['tpverify']) 
{
$error .= "Verification code is incorrect. ";
}
 
if($error === "" && mail($recipient, $subject, $message, "FROM: $email", "-f$email"))
{
header("Location: thankyou.html");
} else {
echo "$error";
}
exit();
 
?>
The above code works but is it safe?
Any help is appreciated

rogie
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: php forms

Post by jackpf »

You may be vulnerable to header injections.

You may want to look at this: viewtopic.php?f=1&t=102493&p=549248&hilit=eregi#p549248
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

Re: php forms

Post by rogie »

Thank you for your reply.

I have done the following changes:

Code: Select all

if(stristr($comments,"http")!=FALSE) // does http appear in the text?
{
   $error .= "No Links please. ";
}
 
if (eregi("%0a", $email) || eregi("%0d", $email) || eregi("Content-Type:", $email) || eregi("bcc:", $email) || eregi("to:", $email) || eregi("cc:", $email))
{
     $error .= "Error with entered data. Please retry.";
}
 
It has changed the email settings to avoid the header injections.

I next problem is that I would need to do the above test for each form field.
How I can set a variable eg $input to be equal to all the form fields so that I can do the above test for each field but with just one line eg

Code: Select all

if (eregi("Content-Type:", $input) || eregi("bcc:", $input) || eregi("to:", $input) || eregi("cc:", $input))
{
     $error .= "Error with entered data. Please retry.";
}
 
$input would be equal to the name, country, telephone etc form fields


Many thanks

Rogie
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: php forms

Post by jackpf »

Well, if you're absolutely sure you want to run it on each field, then you could put $_POST in a foreach() loop.

Also, that code was written a while ago I believe; you may want to use preg_match() in instead of eregi, since ereg functions are now deprecated.
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

Re: php forms

Post by rogie »

Many Thanks for your help

I will look into the preg_match() function

Rogie
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

Re: php forms

Post by rogie »

Hi

Am I there yet? Is my form now more secure?

Code: Select all

<?php
$recipient = "my email here"; 
 
$error = ""; 
 
$name = $_POST['name'];
$email = $_POST['email'];
$subject = "Enquiry"; 
$phone = $_POST['phone'];
$country = $_POST['country'];
$adults = $_POST['adults'];
$children = $_POST['children'];
$age = $_POST['age'];
$arrival_day = $_POST['arrival_day'];
$arrival_month = $_POST['arrival_month'];
$arrival_year = $_POST['arrival_year'];
$departure_day = $_POST['departure_day'];
$departure_month = $_POST['departure_month'];
$departure_year = $_POST['departure_year'];
$comments = $_POST['comments'];
$verification = $_POST['verification'];
 
$message = 
"Name: " . $name . "\n E-mail: " . $email . "\n Telephone: " . $phone . "\n Country: " . $country . "\n\n Number of Adults: " . $adults . "\n Number of Children: " . $children . "\n Age of Children: " . $age . "\n\n Date of Arrival: " . $arrival_day . "," . $arrival_month . "," . $arrival_year . "\n Date of Departure: " . $departure_day . "," . $departure_month . "," . $departure_year . "\n\n Comments: " . $comments . "\n";
 
 
$emailPattern = '/^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$/';
 
if(!preg_match($emailPattern, $email)) 
{
$error = "Incorrect Email.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
if(md5($verification) != $_COOKIE['tpverify']) 
{
$error .= "Verification code is incorrect.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
 
if(stristr($comments,"http")!=FALSE) // does http appear in the text?
{
   $error .= "No Links please.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
 
if (preg_match("%0a", $email) || preg_match("%0d", $email) || preg_match("Content-Type:", $email) || preg_match("bcc:", $email) || preg_match("to:", $email) || preg_match("cc:", $email))
{
     $error .= "Error with entered data. <br /><a href='javascript&#058;history.back();'>Back</a><br />";
}
 
 
 
if($error === "" && mail($recipient, $subject, $message, "FROM: $email", "-f$email"))
{
header("Location: thankyou.html");
} else {
echo "$error";
}
exit();
 
?>
Any comments please

rogie
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: php forms

Post by jackpf »

I think you need to read up on preg_match() documentation a bit more....PCREs need a non-alphanumeric delimiter. Those regexes should be failing....
User avatar
Mordred
DevNet Resident
Posts: 1579
Joined: Sun Sep 03, 2006 5:19 am
Location: Sofia, Bulgaria

Re: php forms

Post by Mordred »

Code: Select all

$verification = $_POST['verification'];
...
if(md5($verification) != $_COOKIE['tpverify'])
Both halves of the check come from the user, it can't work this way. Use $_SESSION instead.
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

Re: php forms

Post by rogie »

Hi

I am obviously out of my depth now. I put together the code from researching many (and I mean many) sites regarding problems with spam emails.

I now don't know how to sort it all out. I was hoping I had actually made it!!!!!!

Can you please direct me to my next step. I am only a beginner at php. I love it but I am scared of this security issue.

Any help at this stage would save me getting grey hair.

Thanks
User avatar
jackpf
DevNet Resident
Posts: 2119
Joined: Sun Feb 15, 2009 7:22 pm
Location: Ipswich, UK

Re: php forms

Post by jackpf »

Store the verification in a session rather than a cookie.

You can read more here.
rogie
Forum Newbie
Posts: 6
Joined: Fri Jul 31, 2009 5:58 am

Re: php forms

Post by rogie »

Hi

Thanks for viewing my query again.

I have read and research sessions!!!!! I have made the following adjustments to the php processing script

Code: Select all

 
<?php
session_start();
 
$recipient = "my email here"; 
 
$error = ""; 
 
$name = $_POST['name'];
$email = $_POST['email'];
$subject = "Enquiry"; 
$phone = $_POST['phone'];
$country = $_POST['country'];
$adults = $_POST['adults'];
$children = $_POST['children'];
$age = $_POST['age'];
$arrival_day = $_POST['arrival_day'];
$arrival_month = $_POST['arrival_month'];
$arrival_year = $_POST['arrival_year'];
$departure_day = $_POST['departure_day'];
$departure_month = $_POST['departure_month'];
$departure_year = $_POST['departure_year'];
$comments = $_POST['comments'];
$verification = $_POST['verification'];
 
$message = 
"Name: " . $name . "\n E-mail: " . $email . "\n Telephone: " . $phone . "\n Country: " . $country . "\n\n Number of Adults: " . $adults . "\n Number of Children: " . $children . "\n Age of Children: " . $age . "\n\n Date of Arrival: " . $arrival_day . "," . $arrival_month . "," . $arrival_year . "\n Date of Departure: " . $departure_day . "," . $departure_month . "," . $departure_year . "\n\n Comments: " . $comments . "\n";
 
 
$emailPattern = '/^[\w-\.]+@([\w-]+\.)+[\w-]{2,4}$/';
 
if(!preg_match($emailPattern, $email)) 
{
$error = "Incorrect Email.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
if(md5($verification) != $_SESSION['verify']) 
{
$error .= "Verification code is incorrect.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
 
if(stristr($comments,"http")!=FALSE) // does http appear in the text?
{
   $error .= "No Links please.<br /><a href='javascript&#058;history.back();'>Back</a><br /> ";
}
 
if (eregi("%0a", $email) || eregi("%0d", $email) || eregi("Content-Type:", $email) || eregi("bcc:", $email) || eregi("to:", $email) || eregi("cc:", $email))
{
     $error .= "Error with entered data. <br /><a href='javascript&#058;history.back();'>Back</a><br />";
}
 
 
 
if($error === "" && mail($recipient, $subject, $message, "FROM: $email", "-f$email"))
{
header("Location: thankyou.html");
} else {
echo "$error";
}
exit();
 
?>
I have also changed the capcha script. It is as follows:

Code: Select all

[<?php
session_start();
header('Content-type: image/jpeg');
$width = 60;
$height = 28;
$my_image = imagecreatetruecolor($width, $height);
imagefill($my_image, 0, 0, 0xA0A0A0);
// add noise
for ($c = 0; $c < 40; $c++){
  $x = rand(0,$width-1);
  $y = rand(0,$height-1);
  imagesetpixel($my_image, $x, $y, 0x404040);
}
$x = rand(1,8);
$y = rand(1,8);
$rand_string = rand(1000,9999);
imagestring($my_image, 5, $x, $y, $rand_string, 0x000000);
$_SESSION['verify']=md5($rand_string);
imagejpeg($my_image);
imagedestroy($my_image);
?>
/code]

Am I there yet   

Thanks for any replies
User avatar
kaisellgren
DevNet Resident
Posts: 1675
Joined: Sat Jan 07, 2006 5:52 am
Location: Lahti, Finland.

Re: php forms

Post by kaisellgren »

That md5() in your verification achieves nothing really. The way you create a random number is poor. Since you are protecting from spam (right?), you don't need cryptographically strong random number generators, but at least use the Mersenne Twister (http://fi.php.net/manual/en/function.mt-rand.php). You should also use letters and potentially other characters in your verification as that increases the number of possibilities.
Post Reply