3

I have the following code which sends emails out.

Is this good/secure enough for a production environment. i.e. will it stop bots, curl scripts sending spam using it, and stop email injections etc etc?

<?php

    require_once('recaptchalib.php');
    $privatekey = "private keys goes here";
    $resp = recaptcha_check_answer ($privatekey,
                                    $_SERVER["REMOTE_ADDR"],
                                    $_POST["recaptcha_challenge_field"],
                                    $_POST["recaptcha_response_field"]);

    if (!$resp->is_valid) {

        // What happens when the CAPTCHA was entered incorrectly
        die ("The reCAPTCHA wasn't entered correctly. Go back and try it again. " .
             "(reCAPTCHA said: " . $resp->error . ")");

    } else {

        require 'class.phpmailer.php';

        //Create a new PHPMailer instance
        $mail = new PHPMailer();

        //Set who the message is to be sent from
        $mail->SetFrom('oshirowanen@localhost.com');

        //Set who the message is to be sent to
        $mail->AddAddress($_POST['email']);

        //Set the subject line
        $mail->Subject = 'subject goes here';

        //Replace the plain text body with one created manually
        $mail->Body = $_POST['message'];

        //Send the message, check for errors
        if(!$mail->Send()) {

            die ("Mailer Error: " . $mail->ErrorInfo);

        } else {

            echo "Message sent!";

        }

    }

?>

So basically, what I am asking is, is the above code safe enough, secure enough, good enough for a production environment?

oshirowanen
  • 15,331
  • 77
  • 181
  • 330

6 Answers6

4

I haven't used php mailer before but it should take care of safety, escaping, etc.
Your code looks good however:

  • I would improve the script by adding an encoding check before sending - for example like this:

    iconv("UTF-8", "UTF-8//IGNORE", $subject_or_message_or_any_string);
    
  • also I wouldn't display the info if the mail failed to send instead of that I would rather use something like:

    if (!$mail->Send())
    {
        LogErrorMessage("Mailer Error: %s", $mail->ErrorInfo);
        die ("Sorry, mail could not be sent");
    }
    
  • Next I would send or log IP address of user that sent the email form - for cases he likes to spam, you can block him easily.

Zaffy
  • 14,842
  • 8
  • 42
  • 70
4

I would suggest 2 more options:

I.) You can put extra input txt fields in your sending form, and then make them hidden (unseen) for the user with css styles, f.e.

<input type="text" id="commentary" style="display: none;">
<!-- OR -->
<input type="text" id="commentary" style="opacity: 0;">
<!-- OR -->
<input type="text" id="commentary" style="position: absolute; left: -100px; top: -100px;">

<!-- 
The trick is, user won't see these forms and WILL NOT FILL THEM. 
And bot will, so you can easily filter them without even using CAPTCHA. 
-->

II.) You can create a list of users (with their IP's, Names, Cookie-ID's, user-ID's, if they sent email when were authorized on the site etc.) and prevent them from sending similar email's several times in a row (in some short period of time). You can also implement some rules to filter spam bots. F.e., if user tries sending too often, then it could be blocked. Another option is to have a "white list" of authorized users, who would be able to send mails with more freedom and wider restrictions.

Al D
  • 67
  • 5
3

For those who doesn't know what header injection (called by OP email injection) is: Even if we assume captcha is uncrackable, a human can fill your form, add some spam comment, and insert a BCC header with thousands of e-mail addresses and your script will send them.

So you should not allow any newlines in any of the headers (to, subject)

PHPMailer takes care of this, here is the relevant part of the code:

$name = trim(preg_replace('/[\r\n]+/', '', $name)); //Strip breaks and trim
if (!$this->ValidateAddress($address)) {
   $this->SetError($this->Lang('invalid_address').': '. $address);

Recaptcha is breakable, and some spam can be send. You are effectively limiting the spam, but if it's important not to allow any spam, then you need a spam filter on the content of the e-mail, as you can never guarantee that the form will not be send by a human, who wants to send some spam messages. Or you can add a limit of messages send from a given IP per hour so you will effectively limit the amount of spam messages that can be send, even if the captcha is cracked or a human is filling it. And you may add a check so the same message content can't not be send to more than X addresses. This is if it's a popular server and it's really important to protect it from sending spam messages; for general use your code is good enough.

Maxim Krizhanovsky
  • 24,757
  • 5
  • 49
  • 85
0

reCaptcha is a good anti-spam script. For E-mail bots check your page source to hide any e-mails readable for bots like something this: Encrypt mailto email addresses with inline JavaScript

Community
  • 1
  • 1
Lkopo
  • 4,560
  • 8
  • 32
  • 59
  • reCaptcha is as ineffective as it is annoying. Too weak, and bots will be able to parse it. Too strong, and your users won't. Also, today there are companies that easily offer **human-powered** captcha solving solutions. So that your captchas are anything but helpful in this case – Madara's Ghost Sep 01 '13 at 06:54
0

Yes, your code should work, you have a captcha so there's no easy way a bot or script can send spam through it, and PHPMailer is already protected against e-mail header injections so you're safe from attackers abusing your contact form to spam other people.

However nothing prevents humans from filling the captchas manually and spamming you that way, unfortunately there's not much you can do against that (maybe limiting the number of messages per IP per day, so that even a manual spammer will only be able to send a few messages per day).

In my opinion, the best way to do it is to eat the spam and filter it afterwards with something like SpamAssassin, that way you can remove the captcha (put some honeypot fields to catch most of the bots) and improve your user experience while still filtering out most of the spam.

  • 1
    reCaptcha has been cracked (and patched) multiple times. For example, in 2012: http://arstechnica.com/security/2012/05/google-recaptcha-brought-to-its-knees/ – Andy Aug 27 '13 at 20:03
0

If you still feel that it is not secure than you can add up more security to it.
in the form add:

<input name="url" style="display:none">

then after this modify code:

else if($_REQUEST['url']){

    require 'class.phpmailer.php';

    //Create a new PHPMailer instance
    $mail = new PHPMailer();

    //Set who the message is to be sent from
    $mail->SetFrom('oshirowanen@localhost.com');

    //Set who the message is to be sent to
    $mail->AddAddress($_POST['email']);

    //Set the subject line
    $mail->Subject = 'subject goes here';

    //Replace the plain text body with one created manually
    $mail->Body = $_POST['message'];

    //Send the message, check for errors
    if(!$mail->Send()) {

        die ("Mailer Error: " . $mail->ErrorInfo);

    } else {

        echo "Message sent!";

    }
Ajay
  • 225
  • 2
  • 8