I do not guarantee that this answer corrects all problems, there may still be other security problems!
This is the function:
bool mail ( string $to , string $subject , string $message [, string $additional_headers [, string $additional_parameters ]] )
What happens is EXACTLY in the last two parameters of mail()
, which is the $additional_headers
and the $additional_parameters
, this fifth parameter is even worse and I won’t even talk about it.
The $additional_headers
informs headers divided by a CRLF (\r\n
) exactly as you are doing at the end of each line.
How to attack:
How the attack works, the best way to protect yourself is to know exactly how the attack works.
This is what you got:
$headers = 'From: '.$email_from."\r\n".
'Reply-To: '.$email_from."\r\n" .
'X-Mailer: PHP/' . phpversion();
Note that if the value of $email_from
for:
[email protected]\r\nBcc:[email protected]
It will cause:
$headers = 'From: [email protected]\r\n'.
Bcc:[email protected]\r\n'. // <<<
'Reply-To: [email protected]\r\n'.
'Bcc:[email protected]\r\n" . // <<<
'X-Mailer: PHP/' . phpversion();
This will be the result.
Proof:
curl ^
-X POST ^
-d "first_name=Nome&telephone=123456789&[email protected]%0ACc:[email protected]&subject=teste3&message=teste3" ^
https://seu-link.com
Note exactly the value of email
, he’s the important:
[email protected]%0ACc:[email protected]
This will cause the field of email
is equal to [email protected]
with the %0A
which is the CR
in HTML URL Encoding (or %0D%0A
) and finally Cc:[email protected]
.
With this someone else will also receive the email, which can even mention more than one.
But because the clean_string
does not work to remove the Cc
which used? Simple, it is case-sensitive, only the str_ireplace
is case-insensitive. The Cc
is different from cc
. In general, Blacklist do not work. It’s much easier to say what’s allowed than to list everything that’s forbidden!
See that this:
function clean_string($string) {
$bad = array("content-type","bcc:","to:","cc:","href");
return str_replace($bad,"",$string);
}
echo clean_string("BCC: [email protected]\r\n Cc: [email protected]");
Return:
BCC: [email protected] Cc: [email protected]
THIS IS USELESS.
How to protect:
Do not use the last argument, nor will I mention it here because I honestly do not know how to protect it. However the fourth argument has salvation, but first I will list all the errors that have the code:
- Lack of CSRF.
- Not treating the CRLF.
- Message text is not being handled.
1. CSRF:
Add the CSRF-Token, simply use a good CSPRNG and save it to the session, for example:
session_start();
if(!isset($_SESSION['CSRF'])){
$_SESSION['CSRF'] = unpack('H*', random_bytes(64))[1];
}
Then add:
<input type="hidden" name="CSRF" value="<?= $_SESSION['CSRF'] ?>">
Then compare both values, which was sent and is stored in the session:
if(hash_equals($_SESSION['CSRF'], $_POST['CSRF']){
// O CSRF é igual!
}
Do not use ===
, timing-Attack. And obviously do not use the ==
, mainly if using a json_decode
/unserialize
, because "qualquer_coisa" == true
is always true
, which is what Laravel 4 (haha).
2. Removing CRLF:
$email_from = str_ireplace(["\r", "\n", ':', ','], '', $email_from);
That way my code would fall by replace below. '( Because in this case it would become:
[email protected]@outra-pessoa.com
I added the :
and ,
so that in the worst case scenario this fails and both are not valid values for email characters, as far as I know.
3. HTML:
The third issue is that it would be ideal to use the htmlentities
, thus:
$email_message = htmlentities($email_message, ENT_QUOTES || ENT_HTML5, 'UTF-8');
This would ensure that no message would possess HTML, mainly <
, >
to prevent the insertion of a <script>
, the same technique to avoid XSS, the parameters used are to define which encoding is used, remember that for this to work you also need to determine the language and format, for example:
$headers .= 'Content-Type: text/html; charset=UTF-8' . "\r\n";
In your case:
Requires PHP 7.0.0+
I believe this would be enough to correct the above three cases, there are still possible improvements, such as checking if it really is an email, if it really is a phone number and the like. All the edits I’ve made are commented on in the code.
<?php
session_start();
// Adicionado geração de CSRF-Token:
if (!isset($_SESSION['CSRF'])) {
$_SESSION['CSRF'] = unpack('H*', random_bytes(64))[1];
}
header('Content-Type: text/html; charset=utf-8');
// Adicionado comaparação de CSRF-Token e todos os campos são necessários serem preenchidos:
if (isset($_POST)
&& hash_equals($_POST['CSRF'], $_SESSION['CSRF'])
) {
$email_to = "mail que recebe";
$email_subject = "Mensagem vindo do site";
$first_name = $_POST['first_name'];
$email_from = $_POST['email'];
$telephone_from = $_POST['telephone'];
$subject = $_POST['subject'];
$comments = $_POST['message'];
// Adicionado remoção de CRLF:
$email_from = str_ireplace(["\r", "\n", ':', ','], '', $email_from);
$email_message = "Mensagem vindo do site\n\n";
$email_message .= 'Name: ' . $first_name . "\n";
$email_message .= 'Email Address: ' . $email_from . "\n";
$email_message .= 'Telephone: ' . $telephone_from . "\n";
$email_message .= 'Subject: ' . $subject . "\n";
$email_message .= 'Message: ' . $comments . "\n";
// Adicionado htmlentitites para mensagem:
$email_message = htmlentities($email_message, ENT_QUOTES || ENT_HTML5, 'UTF-8');
// Adicionado Content-Type:
$headers = 'Content-Type: text/html; charset=UTF-8' . "\r\n";
$headers .= 'From: ' . $email_from . "\r\n";
$headers .= 'Reply-To: ' . $email_from . "\r\n";
$headers .= 'X-Mailer: PHP/' . phpversion();
mail($email_to, $email_subject, $email_message, $headers);
?>
<!-- Message sent! (change the text below as you wish)-->
<div class="container">
<div class="row">
<div class="col-sm-6 col-sm-offset-3">
<div id="form_response" class="text-center">
<img class="img-responsive" src="../imagens/thumbs/mail_sent.png" title="image" alt="imagem" />
<h1>Parabéns!</h1>
<p>Obrigado <b><?= htmlentities($first_name, ENT_QUOTES || ENT_HTML5, 'UTF-8'); ?></b>, a sua mensagem foi enviada</p>
<a class="btn btn-theme" href="">Voltar</a>
</div>
</div>
</div>
</div>
<!--End Message Sent-->
<?php
}
?>
<form id="contact-form" action="contact.php" name="contactform" class="row" method="post">
<div id="input_name" class="col-md-6">
<input type="text" name="first_name" id="name" class="form-control triggerAnimation animated"
data-animate="bounceIn" placeholder="Nome">
</div>
<div id="input_telephone" class="col-md-6">
<input type="text" name="telephone" id="telephone" class="form-control triggerAnimation animated"
data-animate="bounceIn" placeholder="Telefone">
</div>
<div id="input_email" class="col-md-12">
<input type="text" name="email" id="email" class="form-control triggerAnimation animated"
data-animate="bounceIn" placeholder="Email">
</div>
<div id="input_subject" class="col-md-12">
<input type="text" name="subject" id="subject" class="form-control triggerAnimation animated"
data-animate="bounceIn" placeholder="Assunto">
</div>
<div id="input_message" class="col-md-12">
<textarea class="form-control triggerAnimation animated" data-animate="bounceIn" name="message" id="comments"
rows="6" placeholder="Gostaria de ser contactado(a) para mais informações"></textarea>
</div>
<!-- Adicionado campo `CSRF` -->
<input type="hidden" name="CSRF" value="<?= $_SESSION['CSRF'] ?>">
</form>
CSRF Token is missing, possibly that. But, the $headers parameter is extremely dangerous, an attacker can simply insert a " r n" and enter any information. You should at least remove the " r n", since this is the form (CRLF) with which each additional header is divided.
– Inkeliz