Php email insertion not working on my site - Why?

Hi all

I have never been effected by email insertion/injection but someone asked me about it recently so tested a string i found here on site point against my site and no spam emails were sent, I received one copy to the correct address as would normally happen with a form submission.

The issue is I don’t know why the email insertion failed, looking at my code it should have worked, so it kind of makes me worry that i am testing incorrectly and that ny site is wide open to spammers, or that perhaps my code is fine and i am worrying unnecessarily.

Form


<form action="contact.php" method="post" id="contact-form">
	<input type="hidden" name="submitted" value="2"/>
	<fieldset>
		<p>
			<label<?php if(!$fn) echo ' class="error"'; ?> for="name">Name*: </label><input type="text" name="name" id="name" value="<?php echo $name; ?>" /><br />
			<label<?php if(!$em) echo ' class="error"'; ?> for="form-email">Email*: </label><input type="text" name="email" id="form-email" value="<?php echo $email; ?>" /><br />
			<label<?php if(!$ph) echo ' class="error"'; ?> for="form-phone">Phone*: </label><input type="text" name="phone" id="form-phone" value="<?php echo $phone; ?>" /><br />
			<label<?php if(!$pd) echo ' class="error"'; ?> for="comments">Message*: </label><textarea name="comments" id="comments" cols="50" rows="10"><?php echo $comments; ?></textarea><br />
			<label for="butt-space">&nbsp </label>
				<span class="butt-space">
				<span class="left-cheek"><input type="submit" name="submit" id="submit" value="Send" class="button green2" /></span>
				<span class="right-cheek"><input type="reset" name="reset" id="reset" value="Reset" class="button green2" /></span>
				</span>
				<br /> 
		</p>
	</fieldset>
</form>

Checking code


$valid = TRUE;
if (isset ($_POST['submitted'])) {
    foreach($_POST as $key=>$value) {//begin Foreach
      $$key = $value;
    	}//end Foreach
	
function checkEmail($email) {
  $pattern = "/^[A-z0-9\\._-]+"
         . "@"
         . "[A-z0-9][A-z0-9-]*"
         . "(\\.[A-z0-9_-]+)*"
         . "\\.([A-z]{2,6})$/";
  return preg_match ($pattern, $email);
}
function checkLength($string, $min, $max) {
  $length = strlen ($string);
  if (($length < $min) || ($length > $max)) {
    return FALSE;
  } else {
    return TRUE;
  }
}

    $name =trim($name);
	$name =strip_tags($name,'');
    $valid = $fn = checkLength($name, 2, 60);

    $phone =trim($phone);
	$phone =strip_tags($phone,'');
    $ph = checkLength($phone, 6, 60);
////	$valid = $valid && $ph;

    $email =trim($email);
	$email =strip_tags($email,'');
    $em = checkEmail($email);
	$valid = $valid && $em;

	$comments =trim($comments);
	$comments =strip_tags($comments,'');
	//$comments =nl2br($comments);
	
	$pd = checkLength($comments, 5, 600);
	$valid = $valid && $pd;

	
	if ($valid){
		//Calls Email sending function no further checking is done
		$submitted = 3;
		}//$msg = 'email sent';


Any help would be greatly appreciated

You’re not showing enough of the code. I know there are ways to inject extra headers into other fields, but the usual script-kiddie way is to go after the $headers via the user supplied $email.

What input are you using in the $to, $subject, $message [, $additional_headers [, $additional_parameters] ]

Will do, from what i have read, anything that goes in the header needs to be checked and i do use the $email in the reply to field so i guess that is a vulnerability, I am not a hacker either so i am a bit clueless as to how to test for issue.
Thanks for that

I’m pleasantly surprised. (see my corrected previous post) I’ve seen so many scripts that take the user supplied email address and put it into the header’s “From” (and way too often with no validation) even though the From is the script not the user’s email, that it’s refreshing to see this!

The usual script-kiddie technique is to add Bcc: to the $email as that’s the common vulnerability in email scripts. But you are only using the user supplied $email in the message body and the reply-to header after it passes the regex. I’d say you’re safe enough with that.

What you tested for is the usual attempt. i.e. the %OA are newlines needed to tell the email parser it’s a new header.

I’m not a hacker so I don’t know the details but it’s also possible to inject headers into the message body. And AFAIK the subject too.

So even though you’re pretty much set against the usual attack, if you want to improve your chances with other more advanced techniques, I suggest using regex to white-list the other user supplied input as well.

All fields except the email as it kept failing validation on the email

blah%0ASubject:SpammerSubject%0Abcc:victim@domain2.tld

was the sting, obviously with different email addresses

		$to = "me@example.com";
		$subject = "Enquiry by ".$name;
		$message = "Name ".$name."\\r\
";
		$message .= "Email ".$email."\\r\
";
		$message .= "Phone ".$phone."\\r\
";
		$message .= "Message ".$comments."\\r\
";
		$headers = "From: info@example.com\\r\
";
		$headers .= "Reply-To: ".$email."\\r\
";
		$headers .= "Return-Path: info@example.com\\r\
";
		if ( mail($to,$subject,$message,$headers) ) {

Does that help?

What string did you pass it, and into which field?