-1

I found the following contact form script online and I want to find out if it is secure, and if it is not how I might make it more secure. I just went back to the page where I think I got the code a long time ago and I see one commentor said :

"client side validation is only for user conveneicne, it doens't prevent spam, hackers, or annoying web devs. All a hacker has to do is create their own HTML file without javascript. Spam bots wouldn't even use the form they'll just parse it for the id's and send raw packets. Always check input on the server, never trust the user. "

I'm not exactly sure what that means, but hoping if someone sees a vulnerability in the code below it the comment may make more sense :

<?php

$EmailFrom = Trim(stripslashes($_POST['Email']));
$EmailTo = "info@mysite.com";
$Subject = "Customer Inquiry from MySite.com";
$Name = Trim(stripslashes($_POST['Name'])); 
$Tel = Trim(stripslashes($_POST['Tel'])); 
$Email = Trim(stripslashes($_POST['Email'])); 
$Message = Trim(stripslashes($_POST['Message'])); 

// validation
$validationOK=true;
if (!$validationOK) {
  print "<meta http-equiv=\"refresh\" content=\"0;URL=http://www.mysite.com/contact-us-error.php\">";
  exit;
}

// prepare email body text
$Body = "";
$Body .= "Name: ";
$Body .= $Name;
$Body .= "\n";
$Body .= "Tel: ";
$Body .= $Tel;
$Body .= "\n";
$Body .= "Email: ";
$Body .= $Email;
$Body .= "\n";
$Body .= "Message: ";
$Body .= $Message;
$Body .= "\n";

// send email 
$success = mail($EmailTo, $Subject, $Body, "From: <$EmailFrom>");

// redirect to success page 
if ($success){
  print "<meta http-equiv=\"refresh\" content=\"0;URL=http://www.mysite.com/contact-us-success.php\">";
}
else{
  print "<meta http-equiv=\"refresh\" content=\"0;URL=http://www.mysite.com/contact-us-error.php\">";
}
?>

Thanks for taking a look

  • 2
    Define secure. Right now, anybody could use your website to send mass spam on their behalf. – Oliver Charlesworth Sep 12 '13 at 21:23
  • `trim()` - lowercase!! – Black Sheep Sep 12 '13 at 21:24
  • 1
    Hmm, let's examine the validation phase. First, we set the validation flag to `true`. Then we check to see whether it's `false`. If it is, we absolutely refuse to send mail. – Pointy Sep 12 '13 at 21:25
  • It is better to start variables with lowercase characters (it's a convention). – MC Emperor Sep 12 '13 at 21:25
  • Also, you could use php's `header('Location: contact-us-error.php');` instead of printing HTML meta-tags for redirection. – mathielo Sep 12 '13 at 21:30
  • where is $validationOK coming from? also I am not sure if this is secure against cross-site scripting attacks. you might want to look into htmlentities/htmlspecialchars functions. – Maximus2012 Sep 12 '13 at 21:36

1 Answers1

0

You need to understand why some things aren't secure, not just ask people when you don't know.

First of all, you mentioned client-side validation. Are there any constraints you're trying to validate client-side? For instance, is there javascript (or maybe an HTML attribute) that prevents the user from typing more than a certain number of characters in the body of the email?

If so, and if you count this as a security breach, then the page is not secure. If I wanted to abuse your site in this way, I couldn't do it just by visiting the site in the normal way with a browser. But that doesn't stop me from sending whatever I want over the network. I could use something like curl ( http://curl.haxx.se/ ) to send a long request to your server; your server would have no way of knowing it wasn't from a browser, wouldn't check its length, and would send the email.

There's another way an attacker can use the server for something it's clearly not intended for. Namely, they can add extra headers. For instance, suppose they wanted to add the header MyHeader: something malicious. They could send a request in which $_POST['Email'] was the following string:

me@example.com>\r\nMyHeader: something malicious\r\nJunkHeader: junk

Then, the string "From: <$EmailFrom>" would look like this:

From: <me@example.com>
MyHeader: something malicious
JunkHeader: junk>

And those are the headers that would be sent. (I added a line of junk so that the extra > at the end wouldn't appear as part of MyHeader interfere with whatever nefarious plan I was attempting.)

Presumably because of this vulnerability, according to http://uk1.php.net/manual/en/function.mail.php : "The additional_parameters parameter is disabled in safe_mode and the mail() function will expose a warning message and return FALSE when used." (From PHP4.2.3 onwards.)

To fix this, I suppose it is sufficient to check that $EmailFrom contains no newline characters, and refuse to send the email if it does.

David Knipe
  • 3,417
  • 1
  • 19
  • 19
  • Actually it looks like you tried to fix this with the `stripslashes` part. If so, you've got it backwards: you've converted the string `\r\n` to (carriage return) (newline). You wanted something like `addslashes`, which adds backslashes to escape things. Unfortunately this doesn't quite work, because `addslashes` doesn't apply to carriage returns or newlines. But there must be something in the PHP library that does it. – David Knipe Sep 12 '13 at 22:12