Page 1 of 1

php forms

Posted: Fri Jul 31, 2009 6:07 am
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

Re: php forms

Posted: Fri Jul 31, 2009 6:18 am
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

Re: php forms

Posted: Fri Jul 31, 2009 7:47 am
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

Re: php forms

Posted: Fri Jul 31, 2009 9:50 am
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.

Re: php forms

Posted: Fri Jul 31, 2009 4:17 pm
by rogie
Many Thanks for your help

I will look into the preg_match() function

Rogie

Re: php forms

Posted: Fri Jul 31, 2009 5:18 pm
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

Re: php forms

Posted: Sat Aug 01, 2009 9:09 am
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....

Re: php forms

Posted: Sat Aug 01, 2009 11:06 am
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.

Re: php forms

Posted: Sat Aug 01, 2009 4:08 pm
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

Re: php forms

Posted: Sun Aug 02, 2009 12:20 pm
by jackpf
Store the verification in a session rather than a cookie.

You can read more here.

Re: php forms

Posted: Mon Aug 03, 2009 3:33 pm
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

Re: php forms

Posted: Fri Aug 07, 2009 12:08 pm
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.