views:

73

answers:

2

I've finally got this PHP email script working (didn't work on localhost…), but my concern is that it's not safe.

So - is this safe for spamming and any other security pitfalls I'm not aware of?

<?php
$email = '[email protected]';
$subject = 'Notify about stuff';
$notify = $_REQUEST['email'];

if (!preg_match("/\w+([-+.]\w+)*@\w+([-.]\w+)*\.\w+([-.]\w+)*/", $notify)) {
    echo "<h4>Your email address doesn't validate, please check that you typed it correct.</h4>";
    echo "<a href='javascript:history.back(1);'>Back</a>";
}

elseif(mail($email, $subject, $notify)) {
    echo "<h4>Thank you, you will be notified.</h4>";
} else {
    echo "<h4>Sorry, your email didn't get registered.</h4>";
}
?>

Unrelated: is there a PHP function I can use instead of javascript:history.back(1) ?

Edit: the script using filter instead of RegEx

<?php
$email = '[email protected]';
$subject = 'Notify about stuff';
$notify = $_REQUEST['email'];

if (!filter_var($notify, FILTER_VALIDATE_EMAIL)) {
    echo "<h4>This email address ($notify) is not considered valid, please check that you typed it correct.</h4>";
    echo "<a href='javascript:history.back(1);'>Back</a>";
}

elseif(mail($email, $subject, $notify)) {
    echo "<h4>Thank you, you will be notified.</h4>";
} else {
    echo "<h4>Sorry, your email didn't get registered.</h4>";
}
?>
+3  A: 

I don't know if id use $_SERVER['HTTP_REFERER'] to go back. I feel like that could leave you open to attack since it's set via the request. The way to do it would be to use sessions on the previous page. This way you're not dumping untrustworthy data onto your site.

I dont see any security risks, but id like to suggest the use of filter when checking the validity of emails. its much easier than messing with REs.

Galen
+1 for filter. Nice!
Cam
+1 for filter and not trusting the referer! Either use a session as mentioned or redirect to where the user should have came from.
Pier-Luc Gendreau
Thanks, didn't know about the filter. I've updated the original post.
Eystein
+1  A: 

You can't just regexp match an email address against a short regexp pattern if you want to accept all validly formed email addresses and reject all non-valid one. Use a parser (1, 2) that actually implement against the relevant RFCs to check for validity.

Other things you can do is checking HTTP_REFERER to make sure the request came from within your domain as Chacha102 already mentioned. Just note that not all agent send HTTP_REFERER, and that it can be optionally turned off or faked by users.

If you want to go the extra mile to make sure they are giving you a valid email address, you can check for existing DNS record for mail servers at the domain specified (A, MX, or AAAA). And on top of that, you can do callback verification. That's where you connect to the mail server, tell it you want to send to this email address and see if they say OK.

For callback verification, you should note greylisting servers say OK to everything so even that is not a guarantee. Here's some code I used when I needed such a script. It's a patch onto the parser from (1).

    #
    # Email callback verification
    # Based on http://uk2.php.net/manual/en/function.getmxrr.php
    #

    if (strlen($bits['domain-literal'])){
        $records = array($bits['domain-literal']);
    }elseif (!getmxrr($bits['domain'], $mx_records, $mx_weight)){
        $records = array($bits['domain']);
    }else{
        $mxs = array();

        for ($i = 0; $i < count($mx_records); $i++){
            $mxs[$mx_records[$i]] = $mx_weight[$i];
        }

        asort($mxs);

        $records = array_keys($mxs);
    }

    $user_okay = false;
    for ($j = 0; $j < count($records) && !$user_okay; $j++){
        $fp = @fsockopen($records[$j], 25, $errno, $errstr, 2);
        if($fp){
            $ms_resp = "";

            $ms_resp .= send_command($fp, "HELO ******.com");
            $ms_resp .= send_command($fp, "MAIL FROM:<>");

            $rcpt_text = send_command($fp, "RCPT TO:<" . $email . ">");
            $ms_resp .= $rcpt_text;

            $ms_code = intval(substr($rcpt_text, 0, 3));
            if ($ms_code == 250 || $ms_code == 451){ // Accept all user account on greylisting server
                $user_okay = true;
            }

            $ms_resp .= send_command($fp, "QUIT");

            fclose($fp);
        }
    }

return $user_okay ? 1 : 0;
KTC