Make my PHP mailer more secure please!

Scam · 24 Mar 2006 at 13:44

Hi all.

It's been a while since i worked with a guy who set up these type of contact forms for me. He (and people here!) told me how to put lots of funky security in the PHP to stop spam from bots etc.

He gave me a really detailed send.php which included stuff like blocking sends where the users email address is the same as the domain (a typical bot trick it seems) and some more. However trying to make that file work has completely confused me, so i've gone back the the basic version

I know there's some pro's on here, so can anyone make some simple additions to this for me? All it has on is a timer.

Much appreciated!

PHP:

<?

session_start();

if ($_SESSION['lastmailed'] + 60 < time()) {
	
// send mail	
	
$recipient = "[email protected]";
$subject = "my subject goes here";

$sender = stripslashes($_POST[ "email" ]);

$msg .= "Name: ".stripslashes($_POST[ "name" ])."\r\n";
$msg .= "Email: ".stripslashes($_POST[ "email" ])."\r\n";
$msg .= "Subject: ".stripslashes($_POST[ "subject" ])."\r\n";
$msg .= "Message: ".stripslashes($_POST[ "message" ])."\r\n";

//$msg .= "".$_POST[ "" ]."\n";

mail( $recipient, $subject, $msg, "From: $sender" );
header( "location: thankyou.html" );

$_SESSION['lastmailed'] = time();

} else {

   header( "location: form-error.html" );

}
?>

EDIT: I had a read through the thread in the archive and couldnt really make anything work

Craig321 · 24 Mar 2006 at 14:13

You'll be able to find some visual confirmation code on Google which will make it harder for bots to be assigned to it.

Also maybe do some if(empty($blah)) checking so people don't submit blank emails?

Craig.

Dj_Jestar · 24 Mar 2006 at 14:28

You'll need to change a couple things.

Namely, the stripslashes may not be necessary, dependant on your host having mag_quotes enabled, or disabled.

The other problem will be the headers information can quite easily be manipulated in that code.

Remove all new line char's in the headers, then add your own as you are doing now.

Code:

<?php

$msg .= str_replace("\n", '', $_POST['name']) . "\n";

//etc

?>

You will also need to remove any header info found in the message body itself, which is slightly more tricky.

You can try:

Code:

<?php

$msg .= preg_replace("/^To\:/im", '', $_POST['msg']);
//also perform same checks for other header info.. cc: bcc: etc.


?>

but that is not fool proof.

Scam · 24 Mar 2006 at 15:24

Thanks for the advice guys but i'm sorry - that has all pretty much gone straight over my head

lol

Can i get it in 'stupid' please?

robmiller · 24 Mar 2006 at 15:47

I've tried to comment it well so you can see what's going on:

Code:

<?php

// These functions are used for input cleaning later on in the script.
function stripLineBreaks($string) {
	return preg_replace('#(%0A|%0D|\\n+|\\r+)#i', '', $string);
}
function stripHeaders($string) {
	return preg_replace('#(content-type:|to:|cc:|bcc:)#i', '', $string)
}
// This function does a basic validation of an email address
function isValidEmail($string) {
	return preg_match('#^\w[-.\w]*@([-a-z0-9]+\.)+[a-z]{2,4}$#i', $string);
}

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

// Check if the appropriate fields are set
$fieldsSet = empty($_POST['name']) && empty($_POST['email']) && empty($_POST['subject']) && empty($_POST['message']);

// Check if the email address is valid
$validEmail = isValidEmail($_POST['email']);

if(!$withinTimeLimit) {
	die('Whoah, slow down there cowboy!');
}

if(!$fieldsSet) {
	die('You need to enter something in all of the fields!');
}

if(!$validEmail) {
	die('Your email address, she ees not valid!');
}

// Good to go. Let's scrub that input!

// Strip all whitespace from the name, email and subject fields to prevent injection of custom headers
$name = stripLineBreaks($_POST['name']);
$email = stripLineBreaks($_POST['email']);
$subject = stripLineBreaks($_POST['subject']);
// Strip known headers and trim the content of the message to minimise risk of injection
$message = stripHeaders(trim($_POST['message']));

// Build the headers...
$headers = "From: $name <$email>\r\nReply-To: $name <$email>";

// ...and send the mail.
mail('[email protected]', $subject, $message, $headers);

?>

Scam · 24 Mar 2006 at 15:59

That looks brilliant mate thanks so much, but it's giving me an error:

Code:

Parse error: parse error, unexpected '}' in /home/m2lnet/public_html/touchlinetactics/unlive/send.php on line 9

Obviously that is supposed to be there though.. :/

tolien · 24 Mar 2006 at 16:02

Add a semicolon at the end of line 8.

Scam · 24 Mar 2006 at 16:21

tolien said:
Add a semicolon at the end of line 8.

First thing my housemate said when i asked for help.

lol i'm so useless at this.

It works nicely, but the timer doesnt seem to work. Also we changed the

Code:

if(!$fieldsSet) {
	die('You need to enter something in all of the fields!');
}

to

Code:

if($fieldsSet) {
	die('You need to enter something in all of the fields!');
}

'cos it was giving that error every time. Is that right?

? :confused:

Thanks

Scam · 25 Mar 2006 at 00:39

Well i'm thoroughly confused now

I've been trying to work with rob's code and i see how it's supposed to work, but um, it just doesnt

lol

Gonna leave it for tonight, any more help appreciated!

robmiller · 25 Mar 2006 at 03:39

Yeah I sort of forgot to test it

Add the semicolon on line 8 and change the !$fieldsSet to $fieldsSet

Scam · 25 Mar 2006 at 12:45

No problem. I appreciate the help.

Am i right in thinking that this line

Code:

// Check if the appropriate fields are set
$fieldsSet = empty($_POST['name']) && empty($_POST['email']) && empty($_POST['subject']) && empty($_POST['message']);

should be

Code:

// Check if the appropriate fields are set
$fieldsSet = empty($_POST['name']) || empty($_POST['email']) || empty($_POST['subject']) || empty($_POST['message']);

i.e. to mean 'or' ?

Also is there any issue with using

Code:

header( "location: errorpage.php" );

instead of the

Code:

die('Error message here');

as i'd rather show them an error page.

Scam · 25 Mar 2006 at 13:36

Well i answered my own question. I guess without stopping script it carries on and sends it anyway. How can i redirect them as well as killing the script? :confused:

Dj_Jestar · 26 Mar 2006 at 06:34

Code:

<?php

//this will run..
header('Location: page.php');
exit();

//this will not run..
echo 'boo';

?>

Scam · 26 Mar 2006 at 12:36

Thanks.

I also wanted to add 'TT Web Form: *their subject*' as the subject. This seems to work but i'm not sure if it's still doing the 'stripLineBreaks' properly?

Code:

$subject = "TT Web Form: ".stripLineBreaks($_POST['subject']);

Besides that i'm all done, phew!