dpuk44 Posted March 2, 2017 Share Posted March 2, 2017 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> Quote Link to comment Share on other sites More sharing options...
requinix Posted March 2, 2017 Share Posted March 2, 2017 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. 1 Quote Link to comment Share on other sites More sharing options...
dpuk44 Posted March 2, 2017 Author Share Posted March 2, 2017 Thank you Requinix. Your help and advice is much appreciated mate! Quote Link to comment Share on other sites More sharing options...
Jacques1 Posted March 2, 2017 Share Posted March 2, 2017 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); } 1 Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.