What is the best way to turn a secure contact form?

Asked

Viewed 172 times

2

I have a contact form working but it shows crossscripting security failure in Sitelock. Any help overcoming the problem? Thank you

<?php
            header('Content-Type: text/html; charset=utf-8');

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


                // EDIT THE 2 LINES BELOW AS REQUIRED

                $email_to = "mail que recebe";

                $email_subject = "Mensagem vindo do site";


                $first_name = $_POST['first_name']; // required 
                $email_from = $_POST['email']; // required
                                $telephone_from = $_POST['telephone']; // required
                $subject = $_POST['subject']; // required
                $comments = $_POST['message']; // required

                $email_message = "Mensagem vindo do site\n\n";


                function clean_string($string) {
                    $bad = array("content-type","bcc:","to:","cc:","href");
                    return str_replace($bad,"",$string);
                }


                $email_message .= "Name: ".clean_string($first_name)."\n";
                $email_message .= "Email Address: ".clean_string($email_from)."\n";
                                $email_message .= "Telephone: ".clean_string($telephone_from)."\n";
                $email_message .= "Subject: ".clean_string($subject)."\n";
                $email_message .= "Message: ".clean_string($comments)."\n";


                // create email headers

                $headers = 'From: '.$email_from."\r\n".

                'Reply-To: '.$email_from."\r\n" .

                'X-Mailer: PHP/' . phpversion();

                @mail($email_to, $email_subject, $email_message, $headers); 

                ?>



                <?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>
<div id="form_btn" class="col-md-12">
<div class="text-center">
<div id='recaptcha' class="g-recaptcha"
          data-sitekey=""
            data-callback="onSubmit"
          data-size="invisible">
 <script>onload();</script>


<input type="submit" value="Quero ser contactado(a)" id="submit" class="btn btn-theme triggerAnimation animated" data-animate="bounceIn"> 
</div>                                
</div>
<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.

2 answers

4

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:

  1. Lack of CSRF.
  2. Not treating the CRLF.
  3. 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>
  • Inkeliz understood more or less why I’m not specialized but if I put everything together it would be easier to understand. Where should I replace or add in my php?

  • @Gcpt edited to add something "all together", if the form and PHP are on the same page this will be enough.

  • I appreciate your help Inkeliz but it’s not working and has ceased to resend to the thank you page for your contact. When you click to send everything is white. What error?

  • Look at the PHP logs, possibly using an obsolete version (less than PHP 5.6, which is an insecure version that doesn’t even receive security updates) or not filling all the fields, in local tests, with PHP 7.1 in XAMPP, everything is working normally.

  • I’ve already upgraded from PHP 5.6 to 7.1, I’ve done several tests but it still doesn’t work. I updated the snippet in html, you can see (at the end has the recaptcha and Trigger) It will be because of this?

  • Both parts of the code are on the same page?

  • Full view here. https://answall.com/questions/204380/como-correctr-formul%C3%A1rio-contact-safe

  • Is there a problem in the logs? Put the excerpt at the beginning error_reporting(E_ALL); ini_set('display_errors', '1');, change this later. So you can see the problem. It may be that you are using BOM, for example, preventing the header from being sent. By testing the code exactly, assuming it is the same page, it normally returns the "Congratulations!". If the files are on separate pages it is necessary to generate a CSRF-Token on both pages.

  • Inkeliz What I did: In PHP selector I changed display_errors to On, in error_reporting I changed to E_ALL and log_errors to On. In htaccess I put "php_value error_reporting -1 php_flag display_errors On"... and at the beginning of the snippet what you sent. I used phpcoder to confirm the syntax and it is error-free. I upgraded 7.1 and in the bug Metric it is clean. It still doesn’t work. The page continues to remain blank without forwarding to the acknowledgments (css and html inserted on the same contact.php page). Something is blocking the shot but I can’t find the error. In the logs everything seems normal...

  • @Gcpt made some modifications, they are working normally. The only way to "show up" would be for the CSRF-Token to be different or not sending the session cookie or not filling in all fields. I changed this last case, now simply sent any data it will send.

  • Still not working. Thanks anyway for the help. It’s okay but something is blocking.

  • @ Inkeliz how can I contact you by email? Thank you

Show 7 more comments

0

PHP has the function filter_var() to filter specific contents.

In the case of the mail() function it is interesting to use FILTER_SANITIZE_STRING for the body and headers and use a FILTER_SANITIZE_EMAIL specifically for the sender, recipient and other email addresses.

It is very important to be clear what you want to receive in the email. Use the function strip_tags() to remove all unwanted HTML content from the body of the message or, allow only a restricted set of tags necessary to its purpose, thus maliciously preventing the user to insert tags in the body of the email. What would specifically treat Cross Scripting.

Since in security it’s always better to sin excess, I added @Inkeliz’s comment to your code by removing line breaks:

// limpa dados para o corpo da mensagem
function clean_string($string, $extra) {
    $bad = array("content-type","bcc:","to:","cc:","href");
    $tags_permitidas = "<p><br>";

    if (is_array($extra)) {
        $bad = array_merge($bad, $extra);
    }

    $string = filter_var($string, FILTER_SANITIZE_STRING);

    return strip_tags(str_replace($bad,"",$string), $tags_permitidas);
}

// limpa dados para os cabeçalhos
function clean_string_eol($string) {
    return clean_string($string, array("\r", "\n"));
}                


$email_message .= "Name: ".clean_string_eol($first_name)."\n";
$email_message .= "Email Address: ".filter_var($email_from, FILTER_SANITIZE_EMAIL)."\n";
$email_message .= "Telephone: ".clean_string_eol($telephone_from)."\n";
$email_message .= "Subject: ".clean_string_eol($subject)."\n";
$email_message .= "Message: ".clean_string($comments)."\n";
  • I understood the explanation. I will try and check the Sitelock. I’ll come back later. Thanks for the help!

  • What Gustavo suggested is not working. Still a bug.

Browser other questions tagged

You are not signed in. Login or sign up in order to post.