Become a MacRumors Supporter for $50/year with no ads, ability to filter front page stories, and private forums.

toddburch

macrumors 6502a
Original poster
Dec 4, 2006
748
0
Katy, Texas
Two days ago, I couldn't spell PPH. Er, PHP. (ouch - lame).

Anyway, Im getting ready to set up a new website where I will call for people to join up as a member. Automated database update, confirmation click-here email, yada, yada. So, enter PHP and forms.

I've written some code to test the form for email spam injections and do some other validation. I haven't gotten to the MySQL database stuff yet (or even installed it, actually)

Anyone wth PHP skills in this area interested in giving it a code review or a critique for what I have so far?

Thanks, Todd
 
Thanks Fleetwood. (And, thanks for moving this to the proper forum too, if that was you).

I'm trying to accomplish several things here:

1) learn php
2) perform validation on first and last names
3) perform validation on the email address
4) test for spam email injection
5) build in some debug code I can reuse

It's rather long, but I guess that's the nature of the beast.

Thanks, Todd

p.s. I'm not doing anything with the male/female thing yet.
p.p.s. It's ugly as all getout - ignore the ugliness please. I'm in code mode - not aesthetic mode.

PHP:
<?php 

function injected($data) { 
	return eregi("(bcc:|cc:|to:|subject:|from:)" , $data) ;  // Look for bcc, cc, etc. 
}


function valid_name($name) { 
	/* return eregi("^(([a-záàâäãåçéèêëíìîïñóòôöõúùûüßÿ])+ ?)+([a-záàâäãåçéèêëíìîïñóòôöõúùûüßÿ])* *$", $name) ; */ 
	/* echo "-->$name<---" ;  */ 
	/* return eregi("^(([a-záàâäãåçéèêëíìîïñóòôöõúùûüßÿ])+(\-?|( ?)*))+$", $name) ; */ 
	/* return eregi('^(([[:alpha:]])+(\-?|( ?)*))+$', $name) ;  */
	/* return ereg("^(\p{L&})+$\u", $name) ; */ 
	/* return eregi("(.)*([][?/'\"0-9~`!@#$%^&*()_=+{}:;<>,\\])+(.)*$" , $name) ;  */ 

	/*  This final regex is what I came up with.  It was easier to test for invalid characters than for valid 
	    characters, since I don't have preg() available.  The diacritics above would fail.  */   

	return eregi("^[^][?/'\"0-9~`!@#$%^&*()_=+{}:;<>,\\]+$" , $name) ;  
} 

$secret = "ok" ;   // double secret decoder ring code word 
$error = false ;   // assume no errors. 

$fn_class    = "normal" ; 
$ln_class    = "normal" ; 
$email_class = "normal" ; 


if (isset($_POST['reset'])) { 
	foreach ($_POST as $key => $value) { 
		/* echo "key=$key => $value<br />" ;  */ 
		$_POST[$key] = "" ; 
	} 
	/* echo "in reset code<br/>" ; */ 
	$fn_class    = "normal" ; 
	$ln_class    = "normal" ; 
	$email_class = "normal" ; 
} 

else if ( isset($_POST['submitted']) ) { 
	/* echo "Form Submitted<br/>" ;  */ 
	$first_name = trim($_POST['first_name']) ; 
	$last_name  = trim($_POST['last_name']) ; 
	$email      = trim($_POST['email']) ; 
	$message    = trim(stripslashes(($_POST['message']))) ; 

	
	/**  Do Validation.
	*/ 
	
	/** First & Last name validation.  */  
	$first_name = trim($first_name) ; 
	$_POST['first_name'] = $first_name ; 
	
	$last_name  = trim($last_name) ; 
	$_POST['last_name'] = $last_name ; 
	
	if (!valid_name($first_name)) { $fn_class = "error" ; $error = true ; } 
	if (!valid_name($last_name))  { $ln_class = "error" ; $error = true ; } 
	
	/** Email validation.  */  
	if (!eregi("^([[:alnum:]]|_|\.|-)+@([[:alnum:]]|\.|-)+(\.)([a-z]{2,4})$", $email)) {
		/* echo "<b>Invalid EMAIL address:</b> $email" ;  */ 
		$email_class = "error" ; 
		$error = true ; 
	}
	
	/** Look for email injections. */ 
	
	$spam = false ; 
	$table = "<table border='1'><tr><th>Form Field</th><th>Value</th><th>Valid?</th></tr>\n" ; 
	$rowdata = "" ; 
	foreach ($_POST as $key => $value ) { 
		$rowdata = "<tr><td>$key</td><td>$value</td>" ; 
		if (injected($value)) {
			$spam = true ; 
			$rowdata .= "<td style='background:yellow;'>No" ; 
		}
		else { 
			$rowdata .= "<td style='background:green;'>Yes" ; 
		} 
		$rowdata .= "</td></tr>\n" ; 
		if ($key!='secret') { $table .= $rowdata ; } 
	}
	$table .= "</table><br/><br/>\n\n" ; 
 
	
	if ($spam) { 
		echo $table ; 
		$errtext  = "Disallowed data exists in the form.  Please remove anything that looks like an email header." ; 
		$errtext .= "<br/>\nBy the presence of this data, it appears you are attempting to spam using this mail server.<br/>" ; 
		$errtext .= "<br/>\n\nPress your browsers BACK button to continue, reset the form, and start over." ; 
		echo $errtext ; 
		die() ; 
	} 
	
	/** send the email if all is well  */ 
	if (!$error) { 
		$subject = "Todd's User Group Membership Confirmation" ; 

		$body  = "Sign me up!\n" ; 
		$body .= "$first_name $last_name\n" ; 
		$body .= "email=$email\n" ; 
		$body .= "$message\n\n" ; 
		$body .= "Thanks!" ; 

		$add_headers  = 'From: me@example.com' . "\r\n" ; 
		$add_headers .= 'Reply-To: me@example.com' . "\r\n" ;
		$add_headers .= 'X-Mailer: PHP/' . phpversion() . "\r\n" ; 
		$info_sent = true ; 
		/* $result = mail ($email, $subject, $body, $add_headers) ;  */ 
		$confirm = "<html>\n" ; 
		$confirm .= "<head>\n" ;
		$confirm .= "<title>Alert Page</title>\n" ; 
		$confirm .= "</head>\n" ; 
		$confirm .= "<body>\n" ; 
		$confirm .= "<p>Email not sent yet- still in development...<br/><br/>\n\n" ; 
		$confirm .= "headers=$add_headers<br/><br/>\n\nemail=$email<br/><br/>\n\nsubject=$subject<br/><br/>\n\nbody=$body\n" ; 
		$confirm .= "</p>\n</body>\n" ; 
		$confirm .= "</html>" ; 
		
		//print "<br>email (not really) sent: $result" ;   
		echo $confirm ; 
		die() ; 
	}  
}
?> 
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">

<head>
<meta http-equiv="Content Type" content="text/xhtml;charset=iso-8859-1" /> 
<title>Todd's User's Group</title> 
<!-- link type="text/css" rel="stylesheet" media="screen" href="css/todds.css" / --> 
<style> 
.normal { 
color: blue ; 
}

p { 
color: green ; 
} 

.fineprint { 
font-size: .5em ; 
} 

.error {
color: red ; 
} 
</style>
</head>

<body BGCOLOR=#FFFFFF>
<div id="page">
<div id="myform"> 
<form action="<?php $_SERVER['PHP_SELF']; ?>" method="POST"> 
<fieldset> 
<legend accesskey=F>Todd's Membership Form</legend>
<h3 class="normal">Join Today</he>
<p>Join Todd's User's Group and start receiving member
benefits immediately.</p> 

<label class="<?php echo $fn_class ?>" for="first_name">First Name*</label><br />
<input name="first_name" type="text" size="25" id="first_name" value="<?php echo $_POST['first_name'] ?>" /><br /> 

<label class="<?php echo $ln_class ?>" for="last_name">Last Name*</label><br />
<input name="last_name" type="text" size="25" id="last_name" value="<?php echo $last_name ?>" /><br /> 

<label class="<?php echo $email_class ?>" for="email">Email*</label><br />
<input name="email" type="text" size="25" id="email" value="<?php echo $email ?>" /><br /> 

<INPUT type="radio" name="sex" value="Male"> Male<br />
<INPUT type="radio" name="sex" value="Female"> Female<br />

<label class="normal" for="message">Enter your comments or questions</label><br />
<textarea name="message" rows="6" cols="65" id="message"><?php echo $message ?></textarea><br />

<p class="fineprint">Your information will not be sold or shared with others.</p> 

<input class="formButton" name="submitted" type="submit" value="Go" />
<input class="formButton" name="reset"     type="submit" value="Reset Form" />
<input type="hidden" name="secret" value="<?php echo $secret ?>"> 
<p>Problems with this form?  <a href="mailto:me@example.com">Email us</a> 
</fieldset> 
</form> 
</div 

<?php if ($error) { 
	echo "<p class='error'>Please correct the fields in error and resubmit.</p>" ; 
	} ?> 
</div> 
</body>
</html>
 
Learn about how to protect against SQL Injection, its way more important then filtering out spam.

Not the best for performance, but cast all your variables inside your queries. For example:

INSERT INTO tablea (a,b) VALUES ('$id','$email');

Notice the ' around the int of $id. That will prevent some crafty escapes.

Also, make sure you use mysql_real_escape_string() not addslashes()

I have seen so many small sites getting slammed by SQL Injections recently.
 
I was going to suggest you send the output back to browser for your testing before actually sending the e-mail out. But then I saw that's what you're doing already -)

A few things I'd like to mention:

1.
From PHP manual:
Note: preg_match(), which uses a Perl-compatible regular expression syntax, is often a faster alternative to ereg().

So, I'd suggest you switch to preg_match since there's not much difference in the regex within anyway.

IMO, it's better to include the characters you want to allow (whitelisting), rather than filter out the characters you don't want to (blacklisting). One reason I can think of is if you missed on the characters to exclude, you won't get into much troubles. Of course, the other case is true as well, where you may block certain characters that is part of a valid name (for example, the apostrophe is blocked in your expression, which blocks people with name such as O'Reilly).


2.
Why are you assigning your trim()-med values back to $_POST?
Not that it's wrong, but I recommend assigning it to a separate array instead. That way, if for some reason you want to access the original contents of $_POST, you'll still have them down the script.

(And btw, looks like you are doing the trim() on names twice)


3.
For checking e-mail injection, spam, and the likes, look at the comments in PHP manual on mail()


-stndn.
 
Thanks stndn. Good catch on using trim() twice. I had used it on the first_name, and then added it in a common routine, forgetting to remove it from the first place.

The comments in the mail function section on the php website make my head spin. But, I did glean that I can ease off on what I test for for spam injections.

And, I'll ease off on the regex for testing for valid names too. I missed the single quote. Another good catch.

In regards to storing back into $_POST, at first I wasn't sure if I could, thinking it might be a read-only array. So, I tried it. When it worked, I decided to go ahead and leave the "cleaned up" value there.

Again, thanks for looking at this.

Todd

Edit - oh, as far a preg goes - I agree it would better. But, ereg works, and I would have to jack with my php install to add support for it.
 
Register on MacRumors! This sidebar will go away, and you'll see fewer ads.