PHP Email

[whitewater4562000]

Hi,

I was wondering if people could take a look at this php script and help me make it more secure or make tweaks here and there. The script is below:

Code: Select all

<?php
  /*$mail_to = "........";
  $mail_from = "..................";
  $mail_subject = ".........";
  $mail_start = "Message from Website: ";*/
 
  if(!isset($_POST['send']))
      header("location:../unable.html");
  else
      composeEmail();

  function composeEmail()
  {
     $mail_to = ".....";
       $mail_from = ".......";
          $mail_subject = "..........";
          $mail_start = "Message from Website: ";
    
     $name = stripslashes($_POST['fname']);
     //$name = stripslashes('Bob');
     $email = stripslashes($_POST['email']);
     //$email = stripslashes('whitewater456@googlemail.com');
     $telephone = stripslashes($_POST['telephone']);
     //$telephone = stripslashes('01667111111');
     $message = stripslashes($_POST['message']);
     //$message = stripslashes('This is a test to see if things are working');
    
     $isValid = validateEmail($email);
     if(!$isValid)
          header("location:../unable.html");
    
     $message_body = $mail_start."\n\n";
     $message_body .= "Name:  ".$name."\n";
     $message_body .= "Email:  ".$email."\n";
     $message_body .= "Telephone:  ".$telephone."\n\n\n";
     $message_body .= "Message:  ".$message;
    
     //send email
     if(mail($mail_to, $mail_subject, $message_body , "From: ".$mail_from."\r\n"))
          header("location:../thankyou.html");
     //else
          //header("location:../unable.html");
  }  //end of function <composeEmail>
 
  function validateEmail($emailAdd)
  {
     $emailAdd = filter_var($emailAdd, FILTER_SANITIZE_EMAIL);
     if(filter_var($emailAdd, FILTER_VALIDATE_EMAIL))
          return true;
     else
         return false;
  } //end of function <validateEmail>
?>



I am doing validation in javascript before the form is submited but am not sure if i should do more in php as a fail safe and if stripslashes is enough to catch most problems.

UPDATE: Another issue I'm having is if i keep the mail to, from etc variables outside the function it throws an undefined error. I assume this is down to scope but I would assume if the variables are defined at the top of the file everything in the file should be able to see them.

Thank You

[Great Justice]

You don't need stripslashes(), you're only sending a text email not putting anything into a database.
If you want the email sent as HTML, you'll have to pass some headers and escape input using htmlspecialchars() - but really only have to change < to &lt;

Your global variables need to be declared as such inside the function using the "global" operator to see them, *BUT* DON'T DO THAT, rather pass around variables as function arguments.

[Steax]

tl;dr: fatal security flaw may exist.

It's possible for you to get header injections. You could, for example, add in Bcc: headers as part of the "From" header, and any user could easily spam messages out to hundreds of people at a time. As a general idea, though, be aware that the mail function is extremely vulnerable and does not do any sort of sane validation on its own.

The only solution is to properly validate and check each user input as they come in, and judging from your knowledge on globals in PHP, I think you should hold off from working on the mail function for a little while, until you're used to PHP's security quirks. You can also use frameworks like flourish that include pre-secured functionality.

Now, you could be intending to hardcode the From field and all that, but since I don't know for sure, I won't take any chances by not warning you.

[whitewater4562000]

I have just tried submitting the following code in the email field of my contact page and it doesnt get as far as the mail script due to javascript validation

Code: Select all

email@anonymous.xxx%0ATo:email1@who.xxx


The javascript use regular expressions to validate the email address and is called on the submission of the form. The javascript code is

Code: Select all

function validateEmail(email)
     {
        var whitespace = trim(email.value); //email value with whitesapce striped
        var valEmailChars = /^[^@]+@+[^@.]+\.[^@]*\w\w$/ ;
        var invEmailChars = /[\(\)\<\>\,\;\:\\\"\[\]]/ ;
       
        if(email.value == "" || email.value.length == 0)
        {
           email.style.background = '#ffe303';
           return "Please enter an email address";
        }
        else if(!valEmailChars.test(whitespace))
        {
           email.style.background = '#ffe303';
           return "Please enter a valid email address";
        }
        else if(email.value.match(invEmailChars))
        {
           email.style.background = '#ffe303';
           return "Please enter a valid email address";
        }
        else
        {
           email.style.background = '#ffffff';
           return "";
        }
     } //end of function <validateEmail>


With javascript enabled this seems to be enough to stop header injections(?) so would it be best to add this to the php script just in case javascript isn't enabled. Also is email filter (see first post) a better choice for email validation or are regular expression the way to go.

Sorry for being stupid its been a few years since I did any web coding or any real coding at all. Is the injection at the top what to expect I've just copy and pasted it from a website.

[Divinas]

Javascript validation never should be used for security: it can be turned off by a malicious user, or one could post the forged data directly to your php script. You should only use it as a convenience(because it is fast) for your regular users.

[Steax]

As mentioned, only ever use Javascript validation for convenience, never as an actual line of defense. If you want simple convenience, you can just forgo that regex and set the field type="email" and let the browser do the validation. A person who wants to attack your email form (and they will certainly find them; they're key targets for attackers) will just write a script to send requests to the php script hundreds of times a second.

So I also really hope you have some sort of way to throttle the script sending out messages.

(Apologies if I appear terse on the matter; it really is that dangerous a flaw.)

Do note that if you're inserting the email variable as part of the message, you're probably fine. The significant danger is when that goes into the headers parameter (the third one).

Still, it's a great idea to read up on the subject.

[EvanED]

Javascript validation never should be used for security:

I will go one step further. Javascript validation can not be used for security.

it can be turned off by a malicious user

Or, you know, me.

(I don't consider myself malicious, but I browse with JS off and turn it on for sites that refuse to work.)

[Meem1029]

Javascript validation never should be used for security:

I will go one step further. Javascript validation can not be used for security.

Nonsense. It can be used for security, or more specifically with the intent to provide security. It won't actually provide any, but that doesn't mean that people can't use it for that purpose, despite it being worthless.