ctype_print for email injection prevention

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

ctype_print for email injection prevention

Post by matthijs »

Email injection is a big problem, as can be seen by all messeges on different forums from people asking about solutions for the problems they have.
The article on http://www.anders.com/cms/75 has no less then 438 comments at the moment. I have seen many possible solutions, many using regexp to check for newlines. For example:

Code: Select all

$_POST['email'] = preg_replace("/\r/", "", $_POST['email']); 
$_POST['email'] = preg_replace("/\n/", "", $_POST['email']);
Today I was reading about input filtering in some articles in PHP architect and read about the function ctype_print(), which will check for the presence of all printable characters.
From http://builder.com.com/5100-6371_14-5899580.html
The presence of control characters, such as line feeds or tabs, will cause this function to return false.
I was thinking: for emailinjection newlines are necessary. So can't we use ctype_print to check if an email injection attempt is made?

I know, filtering is all about using a whitelist aproach instead of a blacklist approach, so you will always want to check if recieved input is of the type and length you want. So using a good regexp (can anyone recommend a good one?) to check the emailaddress is necessary. But besides that, would there be any other issues I oversee with this function ctype_print?
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

unfortunately, the regex that complies with the RFC 822 standards is.. well, huge!

http://www.ex-parrot.com/~pdw/Mail-RFC822-Address.html

And that is just for the address.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Post by josh »

Yeah I beleive the email injection is only possible with newlines getting into your data somehow, but I could be wrong...


the only situation I could think of that an email injection would be beneficial to an attacker is modification of the "to" header, so you could explicitly check for that although it is probably not ideal to have them playing around with other fields. My advice would be to go for the whitelist approach but if you are going to just block out line returns instead of just replacing the line returns with NULL and carrying on silently I would present some kind of error message to the user
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

It's not explicitly just the "To:" header info, the problem is the header itself is somewhat.. poor, or atleast, open to such ease of manipulation, as can be read here:

http://securephp.damonkohler.com/index. ... _Injection
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Post by josh »

example taken from that page:

Code: Select all

To: recipient@victim.xxx
 Subject: Hum
 From: email@anonymous.xxx
 To:email1@who.xxx
generally the header the attacker is overwriting is the "to" header, as seen above therefore matching for the text "to:" in any of the data that is going into the header is a sure tell sign of email injection, although I could be wrong I have to admit I haven't read the RFC, maybe a valid email could contain that text..


regardless I was not advising the approach, the approach I would almost recommend 100% of the time is using an email validating regex, or when that is not possible check for line breaks in the user input. Like I mentioned in my previous post the attacker probably only wants to modify the to header, but it is best to not let him modify anything at all.
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

...

so the problem is not with the "To:" field, it is with the entire collection of headers..

To: is a field within the header, not it's own header.

The problem, as you have shown with your example as well, is the headers are somewhat sloppy and can be overwritten or manipulated very easily.

So let's look at this from a PHP point of view:

Code: Select all

<?php

function checkAddr ($addr)
{
    return (bool) preg_match('<some fancy regex pattern>', $addr);
}

$to = (checkAddr($_POST['to']) ? $_POST['to'] : 'default@default.com');
$from = (checkAddr($_POST['from']) ? $_POST['from'] : 'default@default.com');

$subject = $_POST['subject'];
$body = $_POST['message'];

?>
That still leaves you open to header manipulation. So saying it is just the "To:" header field is incorrect.

Not to mention the use of the cc: and bcc: fields.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Indeed, Jenk is correct, it's the headers of the mail function. The explanation on http://securephp.damonkohler.com/index. ... _Injection is excellent. A possible solution given there is to search for \r and \n in the fields that end up in the headers.

Code: Select all

<?php 
   $from=$_POST["sender"];
   if (eregi("\r",$from) || eregi("\n",$from)){
     die("Why ?? ");
   }
 ?>
But there are lot's of other suggestions suggested on different sites en forums.
One I found is:

Code: Select all

function isInjection($text)
{

    $text = strtolower($text);

    if  (preg_match ('#(contents*-s*disposition)|
        (bcc:)|(cc:)|
        (contents*-s*transfers*-s*encoding)|
        (mimes*-s*version)|(multiparts*/s*mixed)|
        (multiparts*/s*alternative)|
        (multiparts*/s*related)|
        (replys*-s*to)|
        (xs*-s*mailer)|
        (xs*-s*sender)|
        (xs*-s*uidl)#is',$text)
        )

       { return true; }

    else

       { return false;}

}
Personally I like to use a combination of filters. Validate the emailaddress with a regexp, and check for any suspicous looking data (see function isInjection).

But, my original question remains. What about using ctype_print in some way?

Code: Select all

if (!ctype_print($string) )
{
   return false;
}
I have not seen that function being used and I don't know the ins and outs of this function. So any suggestions about this? Does it return false in any instance of a /r or /n? Would it be a "safe" option to use in this situation? Or are there situations in which the function can be misled?
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Post by josh »

Again I must restate, I never advised checking for the word "to" in the header fields...
User avatar
Jenk
DevNet Master
Posts: 3587
Joined: Mon Sep 19, 2005 6:24 am
Location: London

Post by Jenk »

jshpro2 wrote:the only situation I could think of that an email injection would be beneficial to an attacker is modification of the "to" header, so you could explicitly check for that although it is probably not ideal to have them playing around with other fields.
That's where. Although you mentioned "it is probably not a good idea" your choice of words/phrasing gives the impression that only the To: field should be checked, when infact anything and everything that goes into the email should be validated, even a footer if you use one.
User avatar
Buddha443556
Forum Regular
Posts: 873
Joined: Fri Mar 19, 2004 1:51 pm

Post by Buddha443556 »

I have not seen that function being used and I don't know the ins and outs of this function. So any suggestions about this? Does it return false in any instance of a /r or /n? Would it be a "safe" option to use in this situation? Or are there situations in which the function can be misled?
ctype_print wouldn't be a safe option because it doesn't catch url escaped characters like "%0A".

Let's see if I can summarize the finer points:

1. Avoid user data in the email headers.
2. Barring 1, sanitize user data with a white list such as: s/[^a-zA-Z0-9 ]//g

The more complicate you make security the easier it is to circumvent. Just keep it simple.
d3ad1ysp0rk
Forum Donator
Posts: 1661
Joined: Mon Oct 20, 2003 8:31 pm
Location: Maine, USA

Post by d3ad1ysp0rk »

This email validator is based on rfc 2822 and isn't quite as long as the one posted before:
http://www.ilovejackdaniels.com/php/ema ... alidation/


Also, one of the biggest problems comes when the user can change headers to use a bcc or such.
josh
DevNet Master
Posts: 4872
Joined: Wed Feb 11, 2004 3:23 pm
Location: Palm beach, Florida

Post by josh »

Jenk, I'm sorry if to you it looked like I was suggesting that method, but I specifically worded

you could check explicitly the "to" header but that is probably not ideal


I kind of paraphrased myself there but where in there do you see me suggesting this method be implemented?



I'm not going to discuss this further, we both agree on "do not explicitly check the to header", you misunderstood me... leave it at that



edit:

hmm, regarding url escaped characters, wouldn't that only be an issue if and only if you called url_decode() AFTER checking for line breaks?
User avatar
Buddha443556
Forum Regular
Posts: 873
Joined: Fri Mar 19, 2004 1:51 pm

Post by Buddha443556 »

jshpro2 wrote:hmm, regarding url escaped characters, wouldn't that only be an issue if and only if you called url_decode() AFTER checking for line breaks?
Maybe, however, PHP mail() usually passes off to a Sendmail like program or a SMTP server not a good idea to assume either is secure.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post by matthijs »

Good comments, really helpful.
ctype_print wouldn't be a safe option because it doesn't catch url escaped characters like "%0A".
That's something I'll look in to.
1. Avoid user data in the email headers.
2. Barring 1, sanitize user data with a white list such as: s/[^a-zA-Z0-9 ]//g

The more complicate you make security the easier it is to circumvent. Just keep it simple.
Good advice. If I can, I avoid putting any user input in the headers. I knew about that email regexp from jackdaniels. Seems like a good one, and good to hear it being mentioned here.

There are sooo many regexp's to "validate" emails, it's very hard to tell which ones are solid, which ones are loose, etc. Certainly some of those regexps look very impressive, but without a good understanding of regexp it's hard to tell if there's not a tiny mistake somewhere. I think both mentioned so far look quite safe to use.

Buddha's advice to keep things simple is one I should stick to for a while I think :)
AGISB
Forum Contributor
Posts: 422
Joined: Fri Jul 09, 2004 1:23 am

Post by AGISB »

The solution is very simple.

Just validate any user input!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!


I checked my sites once I heard about mail injection and it did not work at all as I validated all input anyhow and the attack was caught by the validation routines.


Any serious programmer has security in the back of his head and no matter what potion of the site he is programming he at least knows the risks of what can happen.
Post Reply