CODEWITHSUNDEEP
phpemailcode-injection
user1738750
user1738750

Reputation: 205

PHP Simple Header Injection Prevention

I am trying to prevent mail header injection by looping through all the _POST data but the variables are not passing my validation. I have a few "date" fields that include slashes and some of the fields might be blank. Not sure if that would have anything to do with it. Can anyone see the problem with my logic? I keep getting my "fail" message. Thanks.

if(isset($_POST['submit'])) {

    $boolValidateOK = 1;

    function safe( $name ) {
       return( str_ireplace(array( "\r", "\n", "%0a", "%0d", "Content-Type:", "bcc:","to:","cc:" ), "", $name ) );
    }

    foreach($_POST as $value){
        if(!safe($value)){
            $boolValidateOK = 0;
        }else{
            $boolValidateOK = 1;
        }
    }

    if($boolValidateOK == 1){

        $headers  = 'MIME-Version: 1.0' . "\r\n";
        $headers .= 'Content-type: text/html; charset=iso-8859-1' . "\r\n";
        $to...etc
        $subject...etc
        $message...etc

        mail($to, $subject, $message, $headers);

        $success_message = "win";
    }else{
        $error_message = "fail";
    }
}

Upvotes: 1

Views: 1187

Answers (1)

NullUserException
NullUserException

Reputation: 85468

What you're doing is unnecessary. Note PHP's mail() function signature:

bool mail ( string $to , string $subject , string $message 
              [, string $additional_headers [, string $additional_parameters ]] )

The recipient is the second argument. The headers are all in the fourth argument. The contents of $message or $subject are not going to magically "spill over" to the other arguments.

Since your $headers doesn't depend on user input, it doesn't really matter if the user has entered. No such "injection" is possible. All you're doing is butchering the message.

PS: This code is wrong:

foreach($_POST as $value){
    if(!safe($value)){
        $boolValidateOK = 0;
    }else{
        $boolValidateOK = 1;
    }
}

if($boolValidateOK == 1){

Think about it. Let's say all your post values are not "safe," but the last one iterated is. It will then overwrite $boolValidateOK to 1, and that's the value it will retain by the time the loop has ended.

Also, PHP does have a boolean type. If you have a bool, set them to true or false instead of 1 and 0. It's clearer.

Regardless, I would recommend using something like PHPMailer. The native mail() function is very awkward to use.

Upvotes: 3

Related Questions