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:history.back();'>Back</a><br /> ";
}
if(md5($verification) != $_COOKIE['tpverify'])
{
$error .= "Verification code is incorrect.<br /><a href='javascript:history.back();'>Back</a><br /> ";
}
if(stristr($comments,"http")!=FALSE) // does http appear in the text?
{
$error .= "No Links please.<br /><a href='javascript: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: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:history.back();'>Back</a><br /> ";
}
if(md5($verification) != $_SESSION['verify'])
{
$error .= "Verification code is incorrect.<br /><a href='javascript:history.back();'>Back</a><br /> ";
}
if(stristr($comments,"http")!=FALSE) // does http appear in the text?
{
$error .= "No Links please.<br /><a href='javascript: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: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.