Securing a contact form script

Question

Hello! I am just wondering how secure is this contactform script I just made? My teacher was nagging at me a long time ago when I made my contactforms.

if($_SERVER['REQUEST_METHOD'] === 'POST'){

    $myemail  =    "email@adress.com";
    $name      =    $_POST['name'];
    $email    =    $_POST['email'];
    $phone    =    $_POST['phone'];
    $subject  =    $_POST['subject'];
    $comments =    $_POST['comments'];

   if($name == 0 || !preg_match("/([\w\-]+\@[\w\-]+\.[\w\-]+)/", $email) || !preg_match("/^\d{2}(-\d{3}){2}(\d{2})?$/", $phone) || $subject == 0 || $comments == 0){

       $error_message = 'Something was written wrong..';

   } else {

       $message = "Hello!
       Your contact form has been submitted by:
       Name: $name
       E-mail: $email
       Phone: $phone
       Comments: $comments
       End of message";
       mail($myemail, $subject, $message);
       $error_message = 'Your message was sent!';

    }
}

Any suggestions of how to make it secure?

P.S. Securing a Contact Form and Securing a php contact form are both for WordPress and that's not what I am out for.

You might want to post this over on http://codereview.stackexchange.com/ — j08691, Aug 22 '12 at 19:18
Seems like its vulnerable to mail injection, this means sending (spam-) mails via your server by injecting custom headers and a custom body. Can be fixed by stripping \r and \n off the subject and everything else you put unfiltered into your mail's headers. — Daniel M, Aug 22 '12 at 19:21
Perhaps the teacher was referring to human/bot checkers such as [recaptcha](http://www.google.com/recaptcha). — Matt, Aug 22 '12 at 19:22
@DanielM Something like this? `$subject = "$_POST['subject']\r\n";` — Nworks, Aug 22 '12 at 19:27
@Nworks: No, the exact contrary: `$subject = str_replace(array("\r", "\n"), array('', ''), $subject);` You should so some research on this topic to understand this. — Daniel M, Aug 22 '12 at 19:30
@WebnetMobile.com: I'm pretty sure old php versions don't accept that syntax but yes, the current ones do. — Daniel M, Aug 23 '12 at 11:07
@DanielM: maybe, but anyone using anything older than 5.x is simply asking for troubles anyway. — Marcin Orlowski, Aug 23 '12 at 14:08

score 6 · Accepted Answer · answered Aug 22 '12 at 20:41

6

You can use a function to validate the entries such as :

function check_input($data)
 {
    $data = trim($data);
    $data = stripslashes($data);
    $data = htmlspecialchars($data);
    return $data;
 }

And

        $name      =    check_input($_POST['name']);
        $email    =    check_input($_POST['email']);
        $phone    =    check_input($_POST['phone']);
        $subject  =    check_input($_POST['subject']);
        $comments =    check_input($_POST['comments']);

And

     if ($name && $email && $phone && $subject && $comments) {
         Send contact form...

}

and of course you can add captcha to make it more secure.

answered Aug 22 '12 at 20:41

London Boy

387
1
4
14

1

That's terrible advice. Read [The Great Escapism (Or: What You Need To Know To Work With Text Within Text)](http://kunststube.net/escapism/). – deceze Oct 03 '17 at 11:05
@deceze well it is 5 years old...post an updated answer. – M H Nov 10 '17 at 18:17
2

@Michael It was terrible advice 5 years ago. – deceze Nov 10 '17 at 19:06

score 0 · Answer 2 · answered Aug 22 '12 at 19:23

0

There is nothing insecure in your code really beside lack of data validation. You just collect form data and send it out. so the only 'insecurity' is that you would be easily spammed through that form unless any sort of captcha is used. I am not sure at the moment, but it may be possible to trick mail() to add more receipients with crafted $subject, so it would be save to ensure it's oneliner and strip any CRLFs

answered Aug 22 '12 at 19:23

Marcin Orlowski

72,056
11
123
141

So I have to use a CAPTCHA? What do you mean by strip any CRLFs? – Nworks Aug 22 '12 at 19:29
1

He meant what i meant. CRLF = Carriage return / Line feed (\r\n). Stripping them off means to remove them. – Daniel M Aug 22 '12 at 19:32
3

I would also strongly suggest staying away from mail() and forget about it ever existed. It's as dumb as can be (which is a good thing if you know you need that - which is almost never the case). There are lot of dedicated libraries for sending mails which can take care of many of these problems off of your head with building the mail structure or handling attachements or dealing with authenticated smtp, that using mail() should never be even considered. Take a look at PHP Mailer - easy, nice and tested. – Marcin Orlowski Aug 22 '12 at 20:43

Securing a contact form script

2 Answers2

Linked