2
\$\begingroup\$

Here is the code that I am using on a website's contact form:

HTML file:

    <form name="ajax-form" id="ajax-form" action="php-file.php" method="post">
        <label for="name">Name: * 
            <span class="error" id="err-name">please enter name</span>
        </label>
        <input name="name" id="name" type="text" />
        <label for="email">E-Mail: * 
            <span class="error" id="err-email">please enter e-mail</span>
            <span class="error" id="err-emailvld">e-mail is not a valid format</span>
        </label>
        <input name="email" id="email" type="text" />
        <label for="message">Message:</label>
        <textarea name="message" id="message"></textarea>
        <button id="send">Submit</button>
        <div class="error" id="err-form">There was a problem validating the form please check!</div>
        <div class="error" id="err-timedout">The connection to the server timed out!</div>
        <div class="error" id="err-state"></div>
    </form>
    <div id="ajaxsuccess"><h2>Successfully sent!!</h2></div>

PHP file (php-file.php):

<?php

$send_to = "contact@example.com";
$send_subject = "Ajax form ";

$f_name = cleanupentries($_POST["name"]);
$f_email = cleanupentries($_POST["email"]);
$f_message = cleanupentries($_POST["message"]);
$from_ip = $_SERVER['REMOTE_ADDR'];
$from_browser = $_SERVER['HTTP_USER_AGENT'];

function cleanupentries($entry) {
    $entry = trim($entry);
    $entry = stripslashes($entry);
    $entry = htmlspecialchars($entry);

    return $entry;
}

$message = "This email was submitted on " . date('m-d-Y') . 
"\n\nName: " . $f_name . 
"\n\nE-Mail: " . $f_email . 
"\n\nMessage: \n" . $f_message . 
"\n\n\nTechnical Details:\n" . $from_ip . "\n" . $from_browser;

$send_subject .= " - {$f_name}";

$headers = "From: " . $f_email . "\r\n" .
    "Reply-To: " . $f_email . "\r\n" .
    "X-Mailer: PHP/" . phpversion();

if (!$f_email) {
    echo "no email";
    exit;
}else if (!$f_name){
    echo "no name";
    exit;
}else{
    if (filter_var($f_email, FILTER_VALIDATE_EMAIL)) {
        mail($send_to, $send_subject, $message, $headers);
        echo "true";
    }else{
        echo "invalid email";
        exit;
    }
}

?>

Is this code safe? How secure is the email address from users? Am I using best practices in this implementation?

\$\endgroup\$
2
  • 1
    \$\begingroup\$ Does cleanupentries also clean up newlines? Otherwise it would be trivial to inject additional headers into the outgoing mail. \$\endgroup\$ – Mischa Arefiev Nov 16 '13 at 20:37
  • \$\begingroup\$ @MischaArefiev You should post that as an answer (perhaps with a bit of explanation). \$\endgroup\$ – Corbin Nov 17 '13 at 6:45
2
\$\begingroup\$

As htmlspecialchars doesn't seem to clean up newlines in the code, and none of the other two functions does, it is possible to inject any number of headers into your outgoing messages.

Consider this simple code:

<?php
echo "<pre>\n";
$header = "Hello!\r\nMalicious-Header: malicious/value\r\n";

echo "<pre>\n";
echo "From: admin@example.com\n";
echo "To: user@example.com\n";
echo "Good-Header: " . htmlspecialchars($header);
echo "</pre>\n";
?>

Its output is:

From: admin@example.com
To: user@example.com
Good-Header: Hello!
Malicious-Header: malicious/value

Depending on your installation, this may or may not be dangerous, but it is still a good habit to be aware of injection attacks. The general principle is: get rid of special characters not only from your input context but also from your output context. In your case, you place data into the context of RFC mail headers, and in headers, newlines are special characters.

\$\endgroup\$
2
\$\begingroup\$

Your code is safe as far as I can tell. You may consider adding some additional levels of spam checking though to stop invalid requests sooner.

  1. Use a token generated by your server when the form renders, pass that to php-file.php to check when processing the submission, if it isn't in your token list, it didn't come from your server. These tokens should only be used once before they are discarded. It will help you stop Cross Site Request Forgery attempts. It would mean having a storage medium to keep track of the tokens though, so it adds a little bit of overhead to the process.
  2. Honeypot field(s). These are just input fields that you hide with CSS so that a normal user would not fill them in. If they have a value when you process your script you should stop execution.
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.