• 0

Help improving / securing / reviewing some PHP code


Question

Hello,

A few months back i spent ages teaching myself some PHP and producing the contact form shown below. It was really my first proper go at using PHP and i was just wondering whether you Pro's could have a look over it and spot any glaring errors or security issues?

I was also wondering if someone could show me how to speed up parts of it (by looping?) becuase as you can see a lot of the code is repetitive.

Here the code:

//Set blacklists
	$badwords = "/(naughty|words)/i";
	$exploits = "/(content-type|bcc:|cc:|document.cookie|onclick|onload|javascript)/i";
	$bots = "/(Indy|Blaiz|Java|libwww-perl|Python|OutfoxBot|User-Agent|PycURL|AlphaServer|T8Abot|Syntryx|WinHttp|WebBandit|nicebot)/i";

	//Check for any bots
	if(preg_match($bots, $_SERVER['HTTP_USER_AGENT'])) {
		die('<h1>Spam bots are not welcome.</h1>');
	}

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

	if (!$timeLimit) {
		die('<h1>Whoah, slow down there!</h1>
			<p><a href="javascript:history.go(-1)">Please go back and try sending your enquiry again.</a></p>
			');
	}

	//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 ('<h1>The reCAPTCHA text wasn\'t entered correctly</h1>
			 <p><a href="javascript:history.go(-1)">Please go back and try it again.</a></p>
			 ');
	}

	//Check to make sure that the First Name field is not empty, and that it does not contain badwords or exploits
	if(trim($_POST['firstName']) == '' ) {
		$error['firstName'] = "You didn't enter your <b>First Name</b>.";
	} else if (preg_match($badwords, trim($_POST['firstName'])) !== 0 || preg_match($exploits, trim($_POST['firstName'])) !== 0) {
		$error['firstName'] = "You entered a <b>First Name</b> which contains unacceptable or explicit words.";
	} else {
		$firstName = trim(stripslashes(strip_tags($_POST['firstName'])));
		unset($error['firstName']);
	}

	//Check to make sure that the Last Name field is not empty, and that it does not contain badwords or exploits
	if(trim($_POST['lastName']) == '' ) {
		$error['lastName'] = "You didn't enter your <b>Last Name</b>.";
	} else if (preg_match($badwords, trim($_POST['lastName'])) !== 0 || preg_match($exploits, trim($_POST['lastName'])) !== 0) {
		$error['lastName'] = "You entered a <b>Last Name</b> which contains unacceptable or explicit words.";
	} else {
		$lastName = trim(stripslashes(strip_tags($_POST['lastName'])));
		unset($error['lastName']);
	}

	//Check to make sure sure that a valid Email address is submitted
	if(trim($_POST['email']) == '') {
		$error['email'] = "You didn't enter your <b>Email</b> address.";
	} else if (!preg_match('/([a-z0-9])([-a-z0-9._])+([a-z0-9])\@([a-z0-9])([-a-z0-9_])+([a-z0-9])(\.([a-z0-9])([-a-z0-9_-])([a-z0-9])+)*/i', trim($_POST['email'])) || preg_match($badwords, trim($_POST['email'])) !== 0 || preg_match($exploits, trim($_POST['email'])) !== 0) {
		$error['email'] = "You didn't enter a valid <b>Email</b> address.";
	} else {
		$email = trim(stripslashes(strip_tags($_POST['email'])));
		unset($error['email']);
	}

	//Check to make sure that the Company Name field is not empty, and that it does not contain badwords or exploits
	if(trim($_POST['companyName']) == '') {
		$error['companyName'] = "You didn't enter your <b>Company Name</b>.";
	} else if (preg_match($badwords, trim($_POST['companyName'])) !== 0 || preg_match($exploits, trim($_POST['companyName'])) !== 0) {
		$error['companyName'] = "You entered a <b>Company Name</b> which contains unacceptable or explicit words.";
	} else {
		$companyName = trim(stripslashes(strip_tags($_POST['companyName'])));
		unset($error['companyName']);
	}

	//Check to ensure the Telephone Number field is valid
	if(preg_match($badwords, trim($_POST['telephone'])) !== 0 || preg_match($exploits, trim($_POST['telephone'])) !== 0) {
		$error['telephone'] = "You entered a <b>Telephone Number</b> which contains unacceptable or explicit characters.";
	} elseif (trim($_POST['telephone']) == '') {
		$telephone = "Not specified";
	} else {
		$telephone = trim(stripslashes(strip_tags($_POST['telephone'])));
		unset($error['telephone']);
	}

	//Check to ensure the Website field is valid
	if(preg_match($badwords, trim($_POST['website'])) !== 0 || preg_match($exploits, trim($_POST['website'])) !== 0) {
		$error['website'] = "You entered a <b>Website</b> address which contains unacceptable or explicit characters.";
	} elseif (trim($_POST['website']) == '') {
		$website = "Not specified";
	} else {
		$website = trim(stripslashes(strip_tags($_POST['website'])));
		unset($error['website']);
	}

	//Check to make sure a Message was entered
	if(trim($_POST['message']) == '') {
		$error['message'] = "You didn't enter a <b>Message</b>.";
	} else if (preg_match($badwords, trim($_POST['message'])) !== 0 || preg_match($exploits, trim($_POST['message'])) !== 0) {
		$error['message'] = "You entered a <b>Message</b> which contains unacceptable or explicit words.";
	} else {
		$message = trim(stripslashes(strip_tags($_POST['message'])));
		unset($error['message']);
	}

	//Get title and country
	$title = $_POST['title'];
	$country = $_POST['country'];

	//If there are any errors
	if(isset($error)) { 
		echo '<h1>Your Message Has Not Been Sent</h1>';
		echo '<p>Unfortunately your message was <b>not</b> sent as the following errors were detected:</p>';
		echo '<br />';
		echo '<ul>';
		if(isset($error['firstName'])){
			echo '<li> '.$error['firstName'].' </li>';
		}
		if(isset($error['lastName'])){
			echo '<li> '.$error['lastName'].' </li>';
		}
		if(isset($error['email'])){
			echo '<li> '.$error['email'].' </li>';
		}
		if(isset($error['companyName'])){
			echo '<li> '.$error['companyName'].' </li>';
		}
		if(isset($error['telephone'])){
			echo '<li> '.$error['telephone'].' </li>';
		}
		if(isset($error['website'])){
			echo '<li> '.$error['website'].' </li>';
		}
		if(isset($error['message'])){
			echo '<li> '.$error['message'].' </li>';
		}
		echo '</ul>';
		echo '<br />';
		echo '<p>Please <a href="javascript:history.go(-1)">go back and correct these errors.</a></p>';
	}

	//If there are no errors in the error array, send the email
	else if(!is_array($error)) {

And then the email is sent.

Cheers for the help

Link to comment
Share on other sites

1 answer to this question

Recommended Posts

  • 0

//Set blacklists
	$badwords = "/(naughty|words)/i";
	$exploits = "/(content-type|bcc:|cc:|document.cookie|onclick|onload|javascript)/i";
	$bots = "/(Indy|Blaiz|Java|libwww-perl|Python|OutfoxBot|User-Agent|PycURL|AlphaServer|T8Abot|Syntryx|WinHttp|WebBandit|nicebot)/i";

	//Check for any bots
	if(preg_match($bots, $_SERVER['HTTP_USER_AGENT'])) {
	/****
	send a 404 or 302 header instead so they don't come back instead
	****/
		die('<h1>Spam bots are not welcome.</h1>');
	}

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

	if (!$timeLimit) {
		die('<h1>Whoah, slow down there!</h1>
			<p><a href="javascript:history.go(-1)">Please go back and try sending your enquiry again.</a></p>
			');
	}

	//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 ('<h1>The reCAPTCHA text wasn\'t entered correctly</h1>
			 <p><a href="javascript:history.go(-1)">Please go back and try it again.</a></p>
			 ');
	}

	//Check to make sure that the First Name field is not empty, and that it does not contain badwords or exploits
	/****
	1. Instead of a blacklist, use a whitelist. For example a regex that strips everything bar alphanum, basic punctuation and whitespace.
	2. Instead of the exploit list, just strip endlines, this isn't necessary if you use a whitelist that doesn't allow endlines.
	Same applies for the other variables beneath
	****/
	if(trim($_POST['firstName']) == '' ) {
		$error['firstName'] = "You didn't enter your <b>First Name</b>.";
	} else if (preg_match($badwords, trim($_POST['firstName'])) !== 0 || preg_match($exploits, trim($_POST['firstName'])) !== 0) {
		$error['firstName'] = "You entered a <b>First Name</b> which contains unacceptable or explicit words.";
	} else {
		$firstName = trim(stripslashes(strip_tags($_POST['firstName'])));
		unset($error['firstName']);
	}

	//Check to make sure that the Last Name field is not empty, and that it does not contain badwords or exploits
	if(trim($_POST['lastName']) == '' ) {
		$error['lastName'] = "You didn't enter your <b>Last Name</b>.";
	} else if (preg_match($badwords, trim($_POST['lastName'])) !== 0 || preg_match($exploits, trim($_POST['lastName'])) !== 0) {
		$error['lastName'] = "You entered a <b>Last Name</b> which contains unacceptable or explicit words.";
	} else {
		$lastName = trim(stripslashes(strip_tags($_POST['lastName'])));
		unset($error['lastName']);
	}

	//Check to make sure sure that a valid Email address is submitted
	if(trim($_POST['email']) == '') {
		$error['email'] = "You didn't enter your <b>Email</b> address.";
	} else if (!preg_match('/([a-z0-9])([-a-z0-9._])+([a-z0-9])\@([a-z0-9])([-a-z0-9_])+([a-z0-9])(\.([a-z0-9])([-a-z0-9_-])([a-z0-9])+)*/i', trim($_POST['email'])) || preg_match($badwords, trim($_POST['email'])) !== 0 || preg_match($exploits, trim($_POST['email'])) !== 0) {
		$error['email'] = "You didn't enter a valid <b>Email</b> address.";
	} else {
		$email = trim(stripslashes(strip_tags($_POST['email'])));
		unset($error['email']);
	}

	//Check to make sure that the Company Name field is not empty, and that it does not contain badwords or exploits
	if(trim($_POST['companyName']) == '') {
		$error['companyName'] = "You didn't enter your <b>Company Name</b>.";
	} else if (preg_match($badwords, trim($_POST['companyName'])) !== 0 || preg_match($exploits, trim($_POST['companyName'])) !== 0) {
		$error['companyName'] = "You entered a <b>Company Name</b> which contains unacceptable or explicit words.";
	} else {
		$companyName = trim(stripslashes(strip_tags($_POST['companyName'])));
		unset($error['companyName']);
	}

	//Check to ensure the Telephone Number field is valid
	if(preg_match($badwords, trim($_POST['telephone'])) !== 0 || preg_match($exploits, trim($_POST['telephone'])) !== 0) {
		$error['telephone'] = "You entered a <b>Telephone Number</b> which contains unacceptable or explicit characters.";
	} elseif (trim($_POST['telephone']) == '') {
		$telephone = "Not specified";
	} else {
		$telephone = trim(stripslashes(strip_tags($_POST['telephone'])));
		unset($error['telephone']);
	}

	//Check to ensure the Website field is valid
	if(preg_match($badwords, trim($_POST['website'])) !== 0 || preg_match($exploits, trim($_POST['website'])) !== 0) {
		$error['website'] = "You entered a <b>Website</b> address which contains unacceptable or explicit characters.";
	} elseif (trim($_POST['website']) == '') {
		$website = "Not specified";
	} else {
		$website = trim(stripslashes(strip_tags($_POST['website'])));
		unset($error['website']);
	}
	/****
	You'll need to leave endlines here, but as they use different delimiters anyway, it doesn't matter, the exploit list is ineffective in the message body when you're stripping the tags.
	****/
	//Check to make sure a Message was entered
	if(trim($_POST['message']) == '') {
		$error['message'] = "You didn't enter a <b>Message</b>.";
	} else if (preg_match($badwords, trim($_POST['message'])) !== 0 || preg_match($exploits, trim($_POST['message'])) !== 0) {
		$error['message'] = "You entered a <b>Message</b> which contains unacceptable or explicit words.";
	} else {
		$message = trim(stripslashes(strip_tags($_POST['message'])));
		unset($error['message']);
	}

	//Get title and country
	$title = $_POST['title'];
	$country = $_POST['country'];

	//If there are any errors
	if(isset($error)) { 
		echo '<h1>Your Message Has Not Been Sent</h1>';
		echo '<p>Unfortunately your message was <b>not</b> sent as the following errors were detected:</p>';
		echo '<br />';
		echo '<ul>';
		if(isset($error['firstName'])){
			echo '<li> '.$error['firstName'].' </li>';
		}
		if(isset($error['lastName'])){
			echo '<li> '.$error['lastName'].' </li>';
		}
		if(isset($error['email'])){
			echo '<li> '.$error['email'].' </li>';
		}
		if(isset($error['companyName'])){
			echo '<li> '.$error['companyName'].' </li>';
		}
		if(isset($error['telephone'])){
			echo '<li> '.$error['telephone'].' </li>';
		}
		if(isset($error['website'])){
			echo '<li> '.$error['website'].' </li>';
		}
		if(isset($error['message'])){
			echo '<li> '.$error['message'].' </li>';
		}
		echo '</ul>';
		echo '<br />';
		echo '<p>Please <a href="javascript:history.go(-1)">go back and correct these errors.</a></p>';
	}

	//If there are no errors in the error array, send the email
	else if(!is_array($error)) {

Link to comment
Share on other sites

This topic is now closed to further replies.
  • Recently Browsing   0 members

    • No registered users viewing this page.