php Mail function; Is this way of using it safe?

Question

I have a classifieds website, and inside each classified, there is a small form.

This form is for users to be able to tip their "friends":

<form action="/bincgi/tip.php" method="post" name="tipForm" id="tipForm">
Tip: <input name="email2" id="email2" type="text" size="30 />
<input type="submit" value="Skicka Tips"/>
<input type="hidden" value="<?php echo $ad_id;?>" name="ad_id2" id="ad_id2" />
<input type="hidden" value="<?php echo $headline;?>" name="headline2" id="headline2" />
</form>

The form is then submitted to a tip.php page, and here is my Q, is this below code safe, ie is it good enough or do I need to make some sanitations and more safety details?

    $to = filter_var($_POST['email2'], FILTER_SANITIZE_EMAIL);
    $ad_id = $_POST['ad_id2'];
    $headline = $_POST['headline2'];

     $subject = 'You got a tip';

     $message ='Hi. You got a tip: '.$headline.'.\n';

    $headers = 'From: [email protected]\r\n';
    mail($to, $subject, $message, $headers);

I haven't tested the above yet.

Answer 1

Regardless of what filtering you do, you'll need to rate limit the sending of these emails. Even if they look to be from you and have some site-specific text, an automated bot could spam several hundred thousand of them and get some kind of response (and blacklist your email server). Only let them send a handful every hour and you won't cut out legitimate traffic.

Answer 2

It's a good idea to sanitise the input before you use it. Check to ensure the two post variables are in the correct format (e.g. only text or numeric (using Regex or is_numeric etc))

Answer 3

It looks like you have XSS in $ad_id = $_POST['ad_id2']; and $headline = $_POST['headline2'];.

There is a security concern with mail(). You must be careful of CRLF injection \r\n in $headers. in this case $headers is not controlled by the attacker, so you have nothing to worry about. Another point although its called CRLF injection, it could also be called LF injection because a new line is all you really need because SMTP is a forgiving protocol.

Answer 4

You are passing $ad_id and $headline to the HTML only to have it passed right back, unchanged. Since ad_id and headline are not editable in the form, don't put them on the form, keep them on the server. That's the most secure.

Answer 5

If $headline comes from your own database, I would not put the text in a hidden field, but the id of the headline and retrieve the actual text from the database before sending the mail.

That way you can A. simply check if the id is really an integer and B. know for sure only valid headlines get sent; now someone can post your form replacing your headline with any text they want.