Registration Email Form, Secure?

[nightkarnation]

Hey Guys!

I have the following working registration form (the form is in Flash and sends the submitted user variables to PHP)

Here's the working php script:

 

if ($action == "registrationEmail")
{
$date = date("F j, Y");
$name=str_replace("\"", "\\\"", $_POST["Name"]);
$email=str_replace("\"", "\\\"", $_POST["Email"]);
$user_id=str_replace("\"", "\\\"", $_POST["UserId"]);
$sex=str_replace("\"", "\\\"", $_POST["Sex"]);
$birthday=str_replace("\"", "\\\"", $_POST["DateOfBirth"]);
$zip=str_replace("\"", "\\\"", $_POST["Zip"]);
$address=str_replace("\"", "\\\"", $_POST["Address"]);
$phoneOne=str_replace("\"", "\\\"", $_POST["PhoneOne"]);
$phoneTwo=str_replace("\"", "\\\"", $_POST["PhoneTwo"]);
$cell_phone=str_replace("\"", "\\\"", $_POST["Cellphone"]);
$cell_operator=str_replace("\"", "\\\"", $_POST["Operadora"]);
$services=str_replace("\"", "\\\"", $_POST["Services"]);

//send email
if( $email == true )
{
	$sender = $email;
	$receiver = my@email.com";
	$client_ip = $_SERVER['REMOTE_ADDR'];
	$email_body = "Email: $email \n\nIP: $client_ip \n\nName: $name \n\nUserId: $user_id \n\nSex: $sex \n\nDate Of Birth: $birthday \n\nZip: $zip \n\nAddress: $address \n\nPhone One: $phoneOne \n\nPhone Two: $phoneTwo \n\nCell Phone: $cell_phone \n\nCell Operator: $cell_operator \n\nServices: $services \n\nDate: $date";		
	$extra = "From: $sender\r\n" . "Reply-To: $sender \r\n" . "X-Mailer: PHP/" . phpversion();

	//echo "success=yes";

	if( mail( $receiver, "New Buyer Subscriber - $subject", $email_body, $extra ) ) 
	{
		echo "success=yes";
	}
	else
	{
		echo "success=no";
	}
}
}

 

Aside from Header Injection and XSS Prevention

I would like to know if its a good idea to use the str_replace as I did:

$name=str_replace("\"", "\\\"", $_POST["Name"]);

which in some way replaces mysql_real_escape_string that cant be used on this script due to the lack of database connection

 

Any Suggestions?

[BlueSkyIS]

the purpose of mysql_real_escape_string is to prepare a statement for insertion into a mysql database. since you are not inserting this data into a database, why use mysql_real_escape_string?

[Anti-Moronic]

First, the way you have applied the str_replace function is quite inefficient. You should instead use a loop. Like, for example, with $_POST:

 

foreach($_POST as $post){

$postvars[$key] = str_replace("\"", "\\\"", $post);

}

 

If you wanted to opt out certain $_POST vars, you could easily incorporate an array of exclusions:

 

$exlude = array('date','somethingelse');

foreach($_POST as $key => $post){

if(!in_array($key, $exclude){

   $postvars[$key] = str_replace("\"", "\\\"", $post);

}

}

 

Second, no need to use mysql_real_escape_string without interaction with database. Other functions you might be interested in - htmlentities, htmlspecialchars and addslashes.

 

You can then easily modify the loop to apply these changes by editing a single line (as oppose to however many post vars you are cleaning):

 

$exlude = array('date','somethingelse');

foreach($_POST as $key => $post){

if(!in_array($key, $exclude){

   $postvars[$key] = addslashes($post);

}

}

 

You can now simply access any of your clean $_POST variables using: $postvars['key'];

 

Finally, you can wrap the above code in a function and manually return each var.

 

$name = mycleanfunction($_POST['Name']);

 

Hope that helps!

 

[nightkarnation]

Hi Anti-Moronic,

 

Thanks a lot for your useful answer!!

 

If I want to use also htmlentities() to prevent general malicious input.

 

Would this code be correct: (?)

 

foreach($_POST as $post){
$postvars[$key] = str_replace("\"", "\\\"", $post);
$postvars[$key] = htmlentities($post);
}

 

Is this correct?

Thanks a lot in advance for your kind help

 

Cheers!

[BlueSkyIS]

I suggest that you do neither of those functions before sending an email. Neither option is doing anything to increase security, but it sure will make it hard for you to read the email.

[BlueSkyIS]

if you feel the need to worry about email content, i suggest that you worry about header injection. here is some code for that:

 

// Attempt to defend against header injections: 
$badStrings = array("Content-Type:", 
	"MIME-Version:", 
	"Content-Transfer-Encoding:", 
	"bcc:", 
	"cc:");	

// Loop through each POST'ed value and test if it contains 
// one of the $badStrings: 
foreach($_POST as $k => $v){ 
	foreach($badStrings as $v2){ 
		if(strpos($v, $v2) !== false){ 
			header("HTTP/1.0 403 Forbidden"); 
			exit; 
		}
	}
}  

[nightkarnation]

First of all, thanks a lot for your reply BlueSkyIS

 

I already have that prevention on my script and feel safe with that kind of script.

But now I am more worried about general php injection and after htmlentities suggestion, really thought that would help. Why do you say its useless?

 

Thanks again for all the help

[BlueSkyIS]

What is "general php injection"? I mean: What exactly are you worried about?

 

You are sending content in an email. Aside from header injection, what type of content are you concerned about sending in an email? If you make everything htmlentities, or if you replace single slashes with triple slashes, you make the content more difficult to read, but have you made it any safer? Is not using htmlentities like many (most, all?) of us being un-safe?

[Anti-Moronic]

What is "general php injection"? I mean: What exactly are you worried about?

 

You are sending content in an email. Aside from header injection, what type of content are you concerned about sending in an email? If you make everything htmlentities, or if you replace single slashes with triple slashes, you make the content more difficult to read, but have you made it any safer? Is not using htmlentities like many (most, all?) of us being un-safe?

 

he doesn't know does he, that's why he is asking for your advice. Will it make it safer? If not, can he make it safer? If so, how?

 

Not, is it necessary?