Page 1 of 3

ctype_print for email injection prevention

Posted: Fri Dec 23, 2005 3:45 pm
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?

Posted: Fri Dec 23, 2005 8:32 pm
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.

Posted: Fri Dec 23, 2005 10:54 pm
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

Posted: Fri Dec 23, 2005 11:07 pm
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

Posted: Fri Dec 23, 2005 11:22 pm
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.

Posted: Sat Dec 24, 2005 12:03 am
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.

Posted: Sat Dec 24, 2005 2:36 am
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?

Posted: Sat Dec 24, 2005 9:43 am
by josh
Again I must restate, I never advised checking for the word "to" in the header fields...

Posted: Sat Dec 24, 2005 11:02 am
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.

Posted: Sat Dec 24, 2005 12:56 pm
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.

Posted: Sat Dec 24, 2005 1:35 pm
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.

Posted: Sat Dec 24, 2005 1:44 pm
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?

Posted: Sat Dec 24, 2005 3:16 pm
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.

Posted: Sat Dec 24, 2005 4:24 pm
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 :)

Posted: Sun Dec 25, 2005 3:01 am
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.