Jump to content

Is my form script secure


dpuk44

Recommended Posts

Hi, can someone let me know if this script looks secure. I have tried to take as many security measures as possible (within my coding ability):

    <?php
    
    // define variables and set to empty values
    $nameErr = $telephoneErr = $emailErr = $detailsErr = $msgStatus = "";
    $name = $telephone = $email = $details = "";

    // if the button has been pressed
    if ($_SERVER["REQUEST_METHOD"] == "POST") {
      
      if (empty($_POST["name"])) {
        $nameErr = "Name is required";
      } else {
        $name = test_input($_POST["name"]);
        // check if name only contains letters and whitespace
        if (!preg_match("/^[a-zA-Z ]*$/",$name)) {
          $nameErr = "Only letters and white space allowed"; 
        }
      }
      
      if (empty($_POST["telephone"])) {
        $telephoneErr = "Telephone is required";
      } else {
        $telephone = test_input($_POST["telephone"]);
      }
      
      if (empty($_POST["email"])) {
        $emailErr = "Email is required";
      } else {
        $email = test_input($_POST["email"]);
        // check if e-mail address is well-formed
        if (!filter_var($email, FILTER_VALIDATE_EMAIL)) {
          $emailErr = "Invalid email format"; 
        }
      }
      
      if (empty($_POST["details"])) {
        $detailsErr = "Details is required";
      } else {
        $details = test_input($_POST["details"]);  
      }
      
      //there are no errors so we are ok to print the submitted post
      $to = "mymainemail.com";
      $subject = "You have a new message from your website";

      $message = "$name <br> $telephone <br> $email <br> $details";
        
      // Always set content-type when sending HTML email
      $headers = "MIME-Version: 1.0" . "\r\n";
      $headers .= "Content-type:text/html;charset=UTF-8" . "\r\n";

      // More headers
      $headers .= 'From: <webmaster@myemail.co.uk>' . "\r\n";

      if (!empty($nameErr) && !empty($telephoneErr) && !empty($emailErr) && !empty($detailsErr)) {
        $msgStatus = "Failed to send";
      } else {
        mail($to,$subject,$message,$headers);
        $msgStatus = "<span style='color: green;'>Successful</span>";
      }
      
    }
    
    function test_input($data) {
      $data = trim($data);
      $data = stripslashes($data);
      $data = htmlspecialchars($data);
      return $data;
    }
    
    ?>
<form method="post" action="<?php echo htmlspecialchars($_SERVER["PHP_SELF"]); ?>">
              <span class="title">24 hour free callback request</span>
              <div class="form-group">
                <label for="name">Name *</label>
                <input type="text" class="form-control" name="name">
                <span class="error"><?php echo $nameErr; ?></span>
              </div>
              <div class="form-group">
                <label for="telephone">Telephone</label>
                <input type="text" class="form-control" name="telephone">
                <span class="error"><?php echo $telephoneErr; ?></span>
              </div>
              <div class="form-group">
                <label for="email">Email *</label>
                <input type="email" class="form-control" name="email">
                <span class="error"><?php echo $emailErr; ?></span>
              </div>
              <div class="form-group">
                <label>Looking For:</label>
                <div class="checkbox">
                  <label><input type="checkbox" value="Insect Control" name="service">Insect Control</label>
                </div>
                <div class="checkbox">
                  <label><input type="checkbox" value="Rodents & Vermin" name="">Rodents & Vermin</label>
                </div>
                <div class="checkbox disabled">
                  <label><input type="checkbox" value="General Enquiry" name="">General Enquiry</label>
                </div>
              </div>
              <div class="form-group">
                <label for="details">Further Details: *</label>
                <textarea class="form-control" rows="5" name="details"></textarea>
                <span class="error"><?php echo $detailsErr; ?></span>
              </div>
              <div class="form-group">
                <button type="submit" class="btn btn-cta">Contact Us</button><br>
                <span class="error"><?php echo $msgStatus; ?></span>
              </div>
            </form>
Link to comment
Share on other sites

test_input is not right.

 

$data = stripslashes($data);
Using stripslashes is old. The reason for doing it in the first place was a (mis)feature called magic_quotes, however that's long since been removed from PHP so don't do this anymore.

 

$data = htmlspecialchars($data);
Don't use htmlspecialchars indiscriminately like this. It should happen at the last minute just before you put something into HTML. Before then you need to leave the data alone, with only a couple exceptions (like using trim).

Instead your $message line should look like

$message = htmlspecialchars($name) . "
" . htmlspecialchars($telephone) . "
" . htmlspecialchars($email) . "
" . htmlspecialchars($details);
(though there are fancier ways of doing it)

 

And actually, you're being more paranoid than you need to be. Remember your regex check on $name? You already know it contains only letters and whitespace so there's no risk of XSS. On the other hand, while you did validate $email using filter_var() there's no guarantee that it's not also valid HTML (because email addresses can be weird like that) so you do need to escape it.

$message = "{$name}
" . htmlspecialchars($telephone) . "
" . htmlspecialchars($email) . "
" . htmlspecialchars($details);
Speaking of names, I sure hope you aren't expecting any feedback from Conan O'Brien because your form would prohibit his. Really, for names you should just allow everything (remembering to add htmlspecialchars back in) because people have weird names.
  • Like 1
Link to comment
Share on other sites

The form has zero bot protection, so get ready for a lot of spam and possibly people trying to flood your inbox. If you don't want to annoy your customers with CAPTCHAs, that's fine, but you should consider to at least implement basic counter-measures:

  • an invisible trap field which must not be filled; this will stop a lot of the more primitive bots
  • an IP address check against a spam blacklist (like Project Honey Pot); if the IP is fishy, make the visitor solve a CAPTCHA

Calling htmlspecialchars() without the ENT_QUOTES flag and an encoding is risky, purposely omitting it is even riskier. The proper approach is to escape all output data, unless you explicitly want it to contain HTML. Write yourself a wrapper function:

function html_escape($data, $encoding)
{
    return htmlspecialchars($data, ENT_QUOTES | ENT_SUBSTITUTE, $encoding);
}
  • Like 1
Link to comment
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.