Forum Moderators: coopster

Message Too Old, No Replies

php mail and preventing header injection

         

mihomes

6:59 am on Jan 7, 2012 (gmt 0)

10+ Year Member



I'm using the php mail function for a contact form on my site. I was wondering if there is anymore I can do, or should do, to completely prevent header injection or any other type of abuse on the form. Below is a snippet of code I am using to do so.

Mind you, I also use pregmatch on each individual form field to limit it to valid entries... for example first name is "/^[a-z '.]+$/i" and so on for each entry. If any of them fail their pregmatch then I return an error for the user to correct the entry.

I realize I am not filtering out /r, /n, %0a, and %0d which is recommended because when I do so the form returns an error. It posts to itself and upon any error it notifies the user while keeping their original entry in the field to change. What I am really concerned about is my text area fields as I do not use a pregmatch on them and need to allow enter and new line for them, however, my thinking is I can allow those as long as the below covers all grounds for any injection.

Can anyone recommend any changes or suggestions to harden the below any better or is everything pretty much covered?


// remove any possible inections 
foreach($values as $key => $input){
$values[$key] = cleaninjections($input);
}

// perform on each field of the form
function cleaninjections($test)
{
// Remove injected headers
$find = array("/bcc\:/i",
"/content\-type\:/i",
"/mime\-version\:/i",
"/cc\:/i",
"/from\:/i",
"/to\:/i",
"/content\-transfer\-encoding\:/i");
$ret = preg_replace($find, "", stripslashes($test));
return $ret;
}

DeeCee

7:20 am on Jan 7, 2012 (gmt 0)

10+ Year Member



Some checks

 
function check_user_content($content) {
$injection_strings = array('apparently-to', 'cc', 'bcc', 'boundary', 'charset', 'content-disposition',
'content-type', 'content-transfer-encoding', 'errors-to', 'in-reply-to', 'message-id',
'mime-version', 'multipart/mixed', 'multipart/alternative', 'multipart/related',
'reply-to', 'x-mailer', 'x-sender', 'x-uidl');
foreach ($injection_strings as $junk) {
$check = strpos(strtolower($content), $junk);
if ($check !== false) {
return false;
}
}
return true;
}
}

mihomes

7:54 am on Jan 7, 2012 (gmt 0)

10+ Year Member



Thanks DeeCee... I will do some reasearch on those other strings for addition to the script. Regarding my original... ifI were to check for newline, carriage, etc is this the proper format? It looks to be working, however, doing a test I can enter new lines in my textarea field and they are not replaced/removed as I would expect:

"/\/r/i",
"/\/n/i",
"/%0a/i",
"/%0d/i")


My understanding is / is the start and end... and to have one in the string i need to do \/... i of course is case insensitive... correct?

mihomes

7:54 am on Jan 7, 2012 (gmt 0)

10+ Year Member



Thanks DeeCee... I will do some reasearch on those other strings for addition to the script. Regarding my original... ifI were to check for newline, carriage, etc is this the proper format? It looks to be working, however, doing a test I can enter new lines in my textarea field and they are not replaced/removed as I would expect:

"/\/r/i",
"/\/n/i",
"/%0a/i",
"/%0d/i")


My understanding is / is the start and end... and to have one in the string i need to do \/... i of course is case insensitive... correct?

DeeCee

8:20 am on Jan 7, 2012 (gmt 0)

10+ Year Member



You can use any character as the start and end of your pattern, not just a '/'. I usually use '~' or '#' to make it something I would not usually use inside the pattern itself, plus they are easy to recognize. Saves all the "escaping with back-slashes for the limiter character, if used in the pattern. Just see your own back-slashes in the tests with /r..

Actually, your "/r" and "/n" does not mean carriage return and newline. It means just what it shows. A 'slash' with an 'r' or an 'n' after it. You need '\n" (backslash n) and \r (backslash r) to make those control codes.

Note also,

a) if you want to test across newlines, you must remember to set the 'm' modifier. Otherwise preg_match assumes the string you are testing stop at the first newline. This particular thing would prevent the tests you list for newline replacement from working. preg_match doe not look at them without 'm'.

b) I usually suggest to also use the 'u' modifier, meaning UTF-8 character set, if thats what your site is. Otherwise if someone international types into your contact form, the test can go funny, as in standard ascii the multi-byte UTF-8 characters are seen as two individual ascii single-byte, which could mean something entirely different.

c) You are using the 'i' (ignore case) modifier on all patterns. That has no meaning for a test for carriage return and newline. Only on tests including alphas.

mihomes

8:54 am on Jan 7, 2012 (gmt 0)

10+ Year Member



I noticed the \r\n were wrong just too tired I guess.

So add the /m to the newline and carriage and it sounds like I should add /u to everything in the test? I do use utf-8 on most sites, but some do not... will there be an ill affect on those I don't? I'm looking to create a 'template' script I can use on all sites for the contact form...

Below and then the /u for everything? I get what it does, but confused on how it works. Is that something I should be using on all my pregmatch... even something like
if(!preg_match("/^[a-z '.]+$/i",$values['fname']))
where I test a first name entry?

$find = array("/cc\:/i",
"/bcc\:/i",
"/to\:/i",
"/from\:/i",
"/Content\-Type\:/i",
"/Mime\-Version\:/i",
"/Content\-Transfer\-Encoding\:/i",
"/\r/m",
"/\n/m",
"/%0a/m",
"/%0d/m");
$ret = preg_replace($find, "", stripslashes($test));
return $ret;
}


Really appreciate the help... pretty much a novice with this stuff.

DeeCee

9:06 am on Jan 7, 2012 (gmt 0)

10+ Year Member



You can use u modifier on everything if you want. it will not have an effect on everything. Depends on the character set used. UTF-8 includes standard ascii, using only single bytes for those. So in most cases nothing is different. But international characters that cannot be represented in the first 256 values switch to multi-byte sequences. You will have to look up UTF8/UTF16.

Good on the modifiers is to read [php.net...]

mihomes

10:21 pm on Jan 11, 2012 (gmt 0)

10+ Year Member



This is what I have come up with so far. I decided to do newlines and carriage in their own func should I want to replace with something other than 'empty'.

DeeCee... based on your recos I think I have everything covered which I need as entries like 'multipart/relaxed' are already covered by removing content-type. I added the m modifier to /r/n to search through entire strings. I shouldn't need the u modifier for anything.

If you or anyone else care to make suggestions please do so. I'm here to learn and make this cover all the bases needed as well.

function cleaninjections($test)
{
// Remove injected headers
$find = array("/cc\:/i",
"/bcc\:/i",
"/to\:/i",
"/from\:/i",
"/content\-type\:/i",
"/mime\-version\:/i",
"/content\-transfer\-encoding\:/i",
"/content\-disposition\:/i",
"/apparently\-to\:/i",
"/errors\-to\:/i",
"/in\-reply\-to\:/i",
"/reply\-to\:/i",
"/message\-id\:/i",
"/x\-mailer\:/i",
"/x\-sender\:/i",
"/x\-uidl\:/i"
);
$ret = preg_replace($find, "", stripslashes($test));
return $ret;
}

function cleanlines($test)
{
// Remove newlines and carriage returns
$find = array("/\r/m",
"/\n/m",
"/%0a/i",
"/%0d/i",
"/%08/",
"/%09/",
);
$ret = preg_replace($find, " ", stripslashes($test));
return $ret;
}