Email Injection attacks in PHP via mail()

Apparently, spammers are now exploiting a hole, or holes, in multiple PHP scripts which use the mail() API.

The holes are described at the SecurePHP wiki; basically, the script author inserts CGI fields directly into a message template without stripping newlines, and this allows attackers to create new headers, take over the message body, and generally take over the mail message and destinations entirely.

Funnily enough, these are the same holes Ronald F. Guilmette and I found in FormMail 1.9, and described in our Jan 2002 advisory Anonymous Mail Forwarding Vulnerabilities in FormMail 1.9 (PDF) on page 10, Exploitation of email and realname CGI Parameters. Ah, plus ca change…

Worth noting that perl’s venerable taint checking would have spotted these, if it were used.

This entry was posted in Uncategorized and tagged , , , , , , . Bookmark the permalink. Both comments and trackbacks are currently closed.


  1. Posted December 8, 2005 at 23:01 | Permalink


    We’ve been seeing a stupid number of these attacks over the last few weeks. Do you know of anyway of shoving the user /domain into the mail log output on exim? Thousands of request from apache aren’t particularly helpful :(

  2. Posted December 8, 2005 at 23:13 | Permalink

    sorry Michele — exim, not a clue. :(

  3. Posted December 8, 2005 at 23:49 | Permalink

    Damn :(

    Thanks anyway

  4. Posted December 9, 2005 at 02:23 | Permalink

    Hi – that Secure PHP wiki is very useful. However, all these solutions rely on the script authors cleaning up their act. I am a web server administrator and we have users with dodgy mail() calls generating bounces to invalid addresses, etc. We need to be able to trace such generated mails, so I have been working on this:

    This appends various server variables into a mail header for all mails generated with this call. It’s in early stages. I’m hoping other people are also interested in it so something can be put forward to the php-internals list. If so, please get in touch!b

  5. Posted December 9, 2005 at 02:44 | Permalink

    Jon – Is that working or purely experimental?

  6. Posted December 9, 2005 at 02:54 | Permalink

    Jon: two comments:

    First, you need to fix that patch so that the _SERVER hash keys and values are, in turn, sanitised! otherwise a malicious client could possibly still add newlines or other bad data to the message header via those.

    Secondly, it’d be worth adding to the patch a change to the mail() API, so that the “to” and “subject” parameters are always modified to strip newlines. That in itself would fix a lot of the bugs out there without requiring script modification, I think (although I really haven’t looked into it).

  7. Posted December 9, 2005 at 19:19 | Permalink

    Michelle, I’d suggest that it was experimental so far, as it has not been tried on any live services. As Justin says, in principle it could be possible for the _SERVER hash to have it’s keys and/or values tainted to inject bad stuff into mails. As the patch stands, they would have to corrupt the values of the SCRIPT_FILENAME, REQUEST_URI or SERVER_NAME variables.

    I agree Justin that it should sanitize those arguments to the function: In addition, although I haven’t thought the repercussions through properly yet, why not sanitise every entry in the headers array passed? Legitimate use would be one line per entry in the array(), but maybe some innocent code somewhere relies on different behaviour.

    I shall work on some patches to try these ideas out :) If you can think of other good places to mention this, please let me know.

  8. Posted December 9, 2005 at 19:35 | Permalink

    Jon — the PHP developer lists are probably the best bet, I think; and possibly a link from the SecurePHP wiki page.

    > why not sanitise every entry in the headers array passed?

    well, it’s not an array, according to at least. ;) Since it’s just a string, it’d be hard to tell “bad” newline usage from “good”, I think.

  9. Posted December 15, 2005 at 11:21 | Permalink

    That’s a very good point, Justin. For a moment there I fantasised that the function was vaguely more sane than it really is :) Perhaps that’s another change that could be made in an API-breaking future PHP version.