How can i improve my PHP contact form?

Major_Disaster · 18 Oct 2009 at 15:09

Hey Guys,

Been making a new site, and when it came to building the contact form I followed a basic tutorial (using PHP and jquery.validate.pack.js for validation) and then added in reCaptcha for a bit of added security. I would really like to have a secure, standard form that i could use and adapt from now on, so i was wondering whether you could suggest some improvements.

Im reluctant to use pre configured ones online becuase:
a) I dont know how good they actually are,
b) I can actually read and understand how my one works, whereas i cant say the same for the others

I know there are going to be loads of holes, so dont hold back!

Code:

<?php
//If the form is submitted
if(isset($_POST['submit'])) {

	//Check to make sure that the name field is not empty
	if(trim($_POST['contactname']) == '') {
		$hasError = true;
	} else {
		$name = trim($_POST['contactname']);
	}

	//Check to make sure sure that a valid email address is submitted
	if(trim($_POST['email']) == '')  {
		$hasError = true;
	} else if (!eregi("^[A-Z0-9._%-]+@[A-Z0-9._%-]+\.[A-Z]{2,4}$", trim($_POST['email']))) {
		$hasError = true;
	} else {
		$email = trim($_POST['email']);
	}

	//Check to make sure that the telephone number field is not empty
	if(trim($_POST['telephone']) == '') {
		$hasError = true;
	} else {
		$telephone = trim($_POST['telephone']);
	}

	//Check to make sure comments were entered
	if(trim($_POST['message']) == '') {
		$hasError = true;
	} else {
		if(function_exists('stripslashes')) {
			$comments = stripslashes(trim($_POST['message']));
		} else {
			$comments = trim($_POST['message']);
		}
	}

	//If there is no error, send the email
	if(!isset($hasError)) {
		//Recipient email address
		$emailRecipient = '***@***.com'; 
		$emailTo = 'Company <'.$emailRecipient.'>'; 

		//Email subject
		$emailSubject = 'Enquiry for Company';

		// Date
		date_default_timezone_set('Europe/London');
		$date = date('l, d F Y \a\t g:i A', time());

		// Customer callback?
		if ($_POST['checkbox'] == "1") {
			$requestCall = 'Yes';
		} else {
			$requestCall = 'No';
		}

		$callBack = 'Customer requests a call back? <strong>'.$requestCall.'</strong>';

		// Message
		$body = '
		<html>
		<head>
			<title>Enquiry for Company</title>
			
			<style type="text/css">
			body {
			font-family:"Trebuchet MS", Tahoma, Verdana, Arial, Helvetica, sans-serif;
			color:#333;
			}
			
			h1 {
			text-align:center;
			font-weight:bold;
			}
			
			</style>
		</head>
		<body>
			<img src="http://***.com/images/logo-email.png" width="250" height="77">
			<h1><b>Attention: Company</b></h1>
			<p>You have received an enquiry, below, through your website contact form. The message was sent: <i>'.$date.'</i></p>
			<p>Name: <i>'.$name.'</i></p>
			<p>Email: <i>'.$email.'</i></p>
			<p>Telephone Number: <i>'.$telephone.'</i></p>
			<p>Comments:<br><i>'.$comments.'</i></p>
			<p>Call Back: <i>'.$callBack.'</i></p>
		</body>
		</html>
		';

		// To send HTML mail, the Content-type header must be set
		$headers = 'MIME-Version: 1.0' . "\r\n";
		$headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";

		// Additional headers
		$headers .= 'From: Company Website <'.$emailRecipient.'>' . "\r\n";
		$headers .= 'Reply-To: '.$name.' <'.$email.'>' . "\r\n";
		$headers .= 'Return-Path: '.$name.' <'.$email.'>' . "\r\n";
		$headers .= 'X-Mailer: PHP/'.phpversion().'' . "\r\n";
		$headers .= 'X-Sender: '.$emailRecipient.'' . "\r\n";

		// reCaptcha
		require_once('recaptchalib.php');
		$privatekey = "***********";
		$resp = recaptcha_check_answer ($privatekey,
			$_SERVER["REMOTE_ADDR"],
			$_POST["recaptcha_challenge_field"],
			$_POST["recaptcha_response_field"]);
		if (!$resp->is_valid) {
			die ("The reCAPTCHA wasn't entered correctly. <a href =\"javascript:history.back()\">Please go back and try it again.</a>"
			);
		}

		// Send the message
		mail($emailTo, $emailSubject, $body, $headers);
		$emailSent = true;
	}
}
?>

and the HTML

Code:

			<!-- Contact Form Starts -->
			<div id="contact-wrapper">

				<?php if(isset($hasError)) { //If errors are found ?>
					<p><strong>Email Sending Failed!</strong></p>
					<p class="error">Your message was <strong>not</strong> sent. Please check if you've filled all the fields with valid information and send your message again.</p>
				<?php } ?>

				<?php if(isset($emailSent) && $emailSent == true) { //If email is sent ?>
					<p><strong>Email Successfully Sent!</strong></p>
					<p>Thank you <strong><?php echo $name;?></strong> for contacting Company! Your email was successfully sent and we will be in touch with you soon.</p>
				<?php } ?>

				<form method="post" action="<?php echo $_SERVER['PHP_SELF']; ?>" id="contactform">

				<div>
					<label for="name"><strong>Full Name:</strong></label>
					<input type="text" size="55" name="contactname" id="contactname" value="" class="required">
				</div>

				<div>
					<label for="email"><strong>Email Address:</strong></label>
					<input type="text" size="55" name="email" id="email" value="" class="required email">
				</div>

				<div>
					<label for="telephone"><strong>Telephone Number:</strong></label>
					<input type="text" size="55" name="telephone" id="telephone" value="" class="required">
				</div>

				<div>
					<label for="message"><strong>Message:</strong></label>
					<textarea rows="5" cols="55" name="message" id="message" class="required"></textarea>
				</div>

				<div>
					<span><strong>Do you want us to call to discuss your ideas?</strong></span>
					<input type="checkbox" name="checkbox" id="checkbox" value="1">
				</div>

				<!-- reCaptcha Starts -->
				<?php
					require_once('recaptchalib.php');
					$publickey = "**********"; // you got this from the signup page
					echo recaptcha_get_html($publickey);
				?>
				<!-- reCaptcha Ends -->

					<input type="submit" value="Send Message" name="submit">
				</form>
			</div>
			<!-- Contact Form Ends -->

Cheers. Im looking forward to hearing your suggestions and learning some new things...

suarve · 18 Oct 2009 at 17:50

Guess you could use eregi or preg mtach to further validate your message and telephone fields.

You could also improve the security a bit - best guide imo - http://php.robm.me.uk/

Major_Disaster · 19 Oct 2009 at 18:57

Thanks for the reply.

Looks like eregi is deprecated and should be avoided, but the link you provided looks interesting.

Cheers

Major_Disaster · 19 Oct 2009 at 22:00

Ok, so ive been doing a bit of research and have made the following ammendments / improvements, what do you think:

1. Set a timer, so there is a minimum of 60s between sending a message

PHP:

//Start the session
	session_start(); 

	// Check if the user has sent a message in the last sixty seconds
	$timeLimit = $_SESSION['lastMailed'] + 60 < time();

	if (!$timeLimit) {
		die ("Whoah, slow down there cowboy! <a href =\"javascript:history.back()\">Please go back and try it again.</a>");
	}

....

//Time when the last message was sent
		$_SESSION['lastMailed'] = time();

2. Checked for bots

PHP:

	//Check for any bots
	$bots = "/(Indy|Blaiz|Java|libwww-perl|Python|OutfoxBot|User-Agent|PycURL|AlphaServer|T8Abot|Syntryx|WinHttp|WebBandit|nicebot)/i";

	if (preg_match($bots, $_SERVER['HTTP_USER_AGENT'])) {
		die ("<p>Spam bots are not allowed.</p>");
    }

3. Stipped post data

PHP:

	//Check to make sure that the name field is not empty
	if(trim($_POST['contactname']) == '') {
		$hasError = true;
	} else {
		$name = trim(stripslashes(strip_tags($_POST['contactname'])));
	}

	//Check to make sure sure that a valid email address is submitted
	if(trim($_POST['email']) == '')  {
		$hasError = true;
	} else if (!eregi("^[A-Z0-9._%-]+@[A-Z0-9._%-]+\.[A-Z]{2,4}$", trim($_POST['email']))) {
		$hasError = true;
	} else {
		$email = trim(stripslashes(strip_tags($_POST['email'])));
	}

	//Check to make sure that the telephone number field is not empty
	if(trim($_POST['telephone']) == '') {
		$hasError = true;
	} else {
		$telephone = trim(stripslashes(strip_tags($_POST['telephone'])));
	}

	//Check to make sure comments were entered
	if(trim($_POST['message']) == '') {
		$hasError = true;
	} else {
		$comments = trim(stripslashes(strip_tags($_POST['message'])));
		
	}

What do you think? Does that all look correct? Anything else i should be doing?

suarve · 20 Oct 2009 at 09:53

Could create a token while your at it? http://php.robm.me.uk/#toc-CSRF

Major_Disaster · 20 Oct 2009 at 21:21

Thanks for the reply.

Ive read through your link (and many others) and still dont quite get the reasons for a token and how to implement in my (single page) contact form.

Also, how do i stop the whole "resend postdata warning" messages?

Cheers