Form security question

[Bethrezen]

Hi all

 

Ok so as a learning exercise I’ve been having a go at building a contact form, and while building a web form and getting the data from that web form is a relatively simple process securing the form to prevent it being abused by hackers and bots is not so striate forward and I could use a little help

 

I started with a tutorial I found here https://www.w3schools.com/php/php_forms.asp and based off of that I crated the following.

<?php
 
  //Start Session
  session_start();
   
  //define and set variables
  $sent = false;
  $conformation_message = "";
 
  $error = false;
  $name_error_message = "";
  $email_error_message = "";
  $subject_error_message = "";
  $comments_error_message = "";
  $captcha_error_message = "";
 
  //add the variables needed to make the mail() function work.
 
  function input_sanitizer($user_input)
  {
    //strip whitespace (or other characters) from the beginning and end of a string.
    $user_input = trim($user_input);
     
    //returns a string with backslashes stripped off
    $user_input = stripslashes($user_input);
     
    //strip HTML and PHP tags from a string
    $user_input = strip_tags($user_input);
     
    //convert special characters to HTML entities
    //"UTF-8" explicitly specifies the use of UTF-8 encoding needed if running older versions of PHP
    //ENT_QUOTES, tells htmlentities() to encodes both double and single quotes
    $user_input = htmlspecialchars($user_input, ENT_QUOTES, "UTF-8");
 
    return $user_input;
  }
 
  //check if the submit button has been pressed.
  if(isset($_POST['submit']))
  {
    //pass each input through the sanitizer function and save the clean data.
    $name = input_sanitizer($_POST['name']);
    $email = input_sanitizer($_POST['email']);
    $subject = input_sanitizer($_POST['subject']);
    $comments = input_sanitizer($_POST['comments']);
    $captcha = input_sanitizer($_POST['captcha']);
 
    //check if the name field is empty.
    if(empty($_POST['name']))
    {
      $error = true;
      $name_error_message = '<span class="error">Name is required.</span>';
    }
 
    //check if the name field only contains letters and spaces.
    else if (!preg_match("/^[a-z\040]*$/i",$_POST['name']))
    {
      $error = true;
      $name_error_message = '<span class="error">Only letters and spaces are allowed.</span>';
    }
   
    //check if the email field is empty.
    if(empty($_POST['email']))
    {
      $error = true;
      $email_error_message = '<span class="error">Email is required.</span>';
    }
           
    //check if the e-mail address is well-formed.
    else if (!filter_var($_POST['email'], FILTER_VALIDATE_EMAIL))
    {
      $error = true;
      $email_error_message = '<span class="error">Please enter a valid email address.</span>';
    }
           
    //check if the subject field is empty.
    if(empty($_POST['subject']))
    {
      $error = true;
      $subject_error_message = '<span class="error">Subject is required.</span>';
    }
   
    // check if the subject field only contains letters and spaces.
    else if (!preg_match("/^[a-z\040]*$/i",$_POST['subject']))
    {
      $error = true;
      $subject_error_message = '<span class="error">Only letters and spaces are allowed</span>';
    }
   
    //check if the comments field is empty.
    if(empty($_POST['comments']))
    {
      $error = true;
      $comments_error_message = '<span class="error">Comments is required.</span>';
    }
   
    //Insert another check to limit what characters users can input into the comments field.
 
    //check if the captcha field is empty.
    if (empty($_POST['captcha']))
    {
      $error = true;
      $captcha_error_message = '<span class="error">Captcha is required.</span>';
    }
   
    //check if the captcha field only contains positive negative or decimal numbers.
    else if(!preg_match("/-?\d\.*/",$_POST['captcha']))
    {
      $error = true;
      $captcha_error_message = '<span class="error">The captcha field can only contains positive, negative or decimal numbers.</span>';
    }
 
    //check if the stored answer is identical to the users response.
    //(float)$_POST['captcha'] and (integer)$_POST['captcha'] forces $_POST['captcha']
    //to be evaluated as a whole number or as a decimal number instead of as a string.
 
    else if ($_SESSION['total'] !== (integer)$_POST['captcha']
    and $_SESSION['total'] !== (float)$_POST['captcha'])
    {
      $error = true;
      $captcha_error_message = '<span class="error">You answered the question incorrectly.</span>';
    }
 
    else if($error === false and $sent === false)
    {
      //add the mail() function to actually send the mail.
     
      $sent = true;
      $conformation_message = '<p class="sent">Message Sent.</p>';
    }
 
  }
 
  if(!isset($_POST['submit']) or $error === true or $sent === true)
  {
    //Generate Random Numbers
    $value_1 = rand(1,100);
    $value_2 = rand(1,100);
    $value_3 = rand(1,4);
 
    switch ($value_3)
    {
      case 1:
        $question = "What is" ." ". $value_1 . " / " . $value_2 . " Rounded to 2 decimal places ? ";
        $answer = round($value_1 / $value_2, 2);
        break;
 
      case 2:
        $question = "What is" ." ". $value_1 . " * " . $value_2 . " ? ";
        $answer = $value_1 * $value_2;
        break;
 
      case 3:
        $question = "What is" ." ". $value_1 . " + " . $value_2 . " ? ";
        $answer = $value_1 + $value_2;
        break;
 
      case 4:
        $question = "What is" ." ". $value_1 . " - " . $value_2 . " ? ";
        $answer = $value_1 - $value_2;
        break;
    }
 
    //Store/Update The Answer To The Question.
    $_SESSION['total'] = $answer;
  }
?>
 
<!doctype html>
<html>
<head>
<meta charset="utf-8">
<meta name="viewport" content="width=device-width, initial-scale=1">
<title>Contact form test</title>
 
<style>
.error{color:red;}
.sent{color:green;}
</style>
 
</head>
 
<body>
 
<main id="contact">
 
<h1>Contact</h1>
 
<form action="test2.php" method="post" name="contact">
 
<p>Name: <?php echo $name_error_message; ?>
<!-- Prevent the browser from deleting the contents of the field if the form didn’t send due to an error. -->
<br><input type="text" name="name" <?php if(isset($_POST['name']) and $error === true){echo'value="'.$name.'"';}?>></p>
 
<p>Email Address: <?php echo $email_error_message; ?>
<!-- Prevent the browser from deleting the contents of the field if the form didn’t send due to an error. -->
<br><input type="text" name="email" <?php if(isset($_POST['email']) and $error === true){echo'value="'.$email.'"';}?>></p>
 
<p>Subject: <?php echo $subject_error_message; ?>
<!-- Prevent the browser from deleting the contents of the field if the form didn’t send due to an error. -->
<br><input type="text" name="subject" <?php if(isset($_POST['subject']) and $error === true){echo'value="'.$subject.'"';}?>></p>
 
<p>Comments: <?php echo $comments_error_message; ?>
<!-- Prevent the browser from deleting the contents of the field if the form didn’t send due to an error. -->
<br><textarea name="comments"><?php if(isset($_POST['comments']) and $error === true){echo $comments;}?></textarea></p>
 
<p><?php echo $question; echo $captcha_error_message; ?>
<br><input type="text" name="captcha"></p>
 
<p><input type="submit" name="submit" value="Submit"></p>
 
<?php if(isset($_POST['submit']) and $sent === true){echo $conformation_message;} ?>
 
</form>
 
</main>
 
</body>
</html>

Now I know this contact form isn’t complete and won't actually send email just yet because there are still a couple of bits missing this is deliberate because I'm not at that point yet, I wanted to try and figure out the form security first because it's important and if you don’t do it correctly then that can lead to all sorts of issues.

 

While what i have right now is a reasonable start I am aware that there are still a few issues with it, and there are some additional things that can be done to make it harder to attack, like for example blocking submissions that where not generated by my form therefore preventing third-party scripts wailing away at my form from a different server which is what I'm trying to figure out now.

 

Ok so this tutorial https://css-tricks.com/serious-form-security/ describes one way of achieving this the problem with the method described as some of the users who commented on this rightly point out is that there is nothing to stop the attacker getting the session key including it in the submission there by circumventing the check one preposed solution to this is to check the HTTP Referrer Header the issue there is that the HTTP Referrer Header can be spoofed which would allow the attacker to once again by pass the check which is not really ideal.

 

so I’m wondering is if there is perhaps another way of preventing the bad guys third-party scripts wailing away at my form from a different server which the can’t be so easily bypass.

 

would something like the lock em up method described here http://green-beast.com/blog/?p=144 work?

Edited April 29, 2017 by Bethrezen

[Jacques1]

It's great that you want to secure your forms. However, there are fundamental problems with your implementation, and a lot of them probably have to do with the fact that w3schools is a very poor source.

 

The “sanitizer” function in your code makes no sense. There is no magical “cleaning” procedure for data. Security always depends on the specific context. When you blindly apply a set of functions to all input, then you're more likely to damage the data than to increase your security. And that's indeed what happens in your code: stripslashes() blindly removes all backslashes, regardless of whether they're important or not. Why would you do that? The last time this function made any sense at all was in the late 90s when PHP still had Magic Quotes (a bad concept by itself). And strip_tags() mangles all strings which resemble HTML, even when they're perfectly valid. For example, the string “I <3 PHP” becomes “I ”, because the function thinks that “<” is the beginning of an HTML element. This is obviously misguided.

 

A much better rule of thumb is: Validate the input, escape the output. So you check the incoming data, but you don't change it. And when you generate your output (i. e. the HTML page), you apply the right functions for the specific context (htmlspecialchars() works for most HTML contexts).

 

As to your other questions:

• Trying to block requests from other sites doesn't increase security and is close to impossible. Don't waste your time with this. Accept the fact that anybody can send requests to your site, and there's nothing wrong with it as long as the requests themselves are valid.
• Use a professional CAPTCHA like Google's reCAPTCHA instead of trying to invent your own. Bots are definitely smart enough to solve math problems; they're actually better at this than humans.
• If you're worried about spam in particular, get a proper spam filter. This will catch a lot of unwanted requests.
• There are several projects dedicated to monitoring malicious bot activity: Stop Forum Spam, Project Honey Pot, Spamhaus. You can use their databases for free (but don't just block every IP address on the list; sometimes they accidentally record legitimate users).

[benanamen]

One other thing. The tutorial you linked to was written 8 years ago. If you're going to use tutorials, make sure they are fairly current. Current still does not mean they are correct though.

[Bethrezen]

Security always depends on the specific context. When you blindly apply a set of functions to all input, then you're more likely to damage the data than to increase your security.

 

I get what you are saying and context is important, and if the form was submitting data to a database then you would be correct and this would cause issues, but in the context of a basic contact form all you want 99.9% of the time is plain text and standard punctuation so you should be striping out anything that shouldn’t be there (like for example html and php tags) while at the same time not mangling the message which is what I’m trying to achieve here

 

And that's indeed what happens in your code: stripslashes() blindly removes all backslashes, regardless of whether they're important or not. Why would you do that? The last time this function made any sense at all was in the late 90s when PHP still had Magic Quotes (a bad concept by itself).

 

With regards to stripslashes() I can only see this as being an issue, within the message field within the other fields I fail to see how this would cause problems because no other field should contain any backslashes anyway, now if there is a reasonable expectation that the users sending a message via your form would need to use backslash character then obviously you would want to omit the message field from having backslashes striped to prevent issues.

 

strip_tags() mangles all strings which resemble HTML, even when they're perfectly valid. For example, the string “I <3 PHP” becomes “I ”, because the function thinks that “<” is the beginning of an HTML element. This is obviously misguided.

 

Again I get what you are saying but if you aren’t expecting code in your emails then removing html and php tags is appropriate as it prevents the execution of any code contained within those tags.

 

Take <script>alert("HELLO");</script> for example without any sanitization this would execute when you opened the email, if you are using html email that is.

 

Now lets supposed that what was contained in the script tags wasn’t something benign like a simple alert() then conceivably you could end up executing a script based virus when you open the email, by removing the unexpected script tags you stop that from happening.

 

Obviously if you where expecting people to be submitting code then this as you point out that could be a problem, but in my case I’m not expecting code of any kind so I want to make absolutely sure that any code that is present can’t execute.

 

So as far as I’m aware the only way this would cause a problem in my case is if the user made a typo when writing there message in which case you would have a part of the message mangled as you point out, but there isn’t a simple way to deal with that, the best you could do to ensure that the message doesn’t get mangled is to not run the message field through the sanitization function and instead do a white list validation check that will cause the form to throw an error if the user types any characters that are not allowed then prevent the email being sent if the form contains errors.

 

With regards to trim() this prevents things like entering a space for the name field because if you try to do that then the space will get removed and you will get an error because the field is blank, granted this doesn’t stop people just entering a single letter but than can be dealt with relatively simply by making sure that people enter a name of at least 3 charters and no longer then say 20 charters, so I don’t see how this would cause problems either.

 

So you see I’m not applying this stuff blindly I’m applying this stuff deliberately because I want the variables that I use to build the email to only contain expected and valid data.

Edited July 27, 2017 by Bethrezen

[Bethrezen]

Trying to block requests from other sites doesn't increase security and is close to impossible. Don't waste your time with this. Accept the fact that anybody can send requests to your site, and there's nothing wrong with it as long as the requests themselves are valid.

 

I see well if you can’t stop submissions that don’t come from your form can you at least stop bad guys from injecting headers like for example CC BCC to stop them from hijacking your form and using that to send spam to other people ?

[ginerjm]

Are you building the mail's headers from any of the form's inputs?  If not then I hope you realize that a hacker can't do that.

[Sepodati]

You ask for advice and then shit all over it?

 

Stop assuming what the user will write and using unnecessary functions and make WHATEVER I write safe to display in whatever context you're displaying it in.

 

If I want to write <script>some code();</script> in my message, then display it safely, just like these forums do. Don't assume my message and delete part of it.

Edited July 27, 2017 by Sepodati

[Jacques1]

Bethrezen: You have fundamental misconceptions about security, and you don't seem to get the essence of what I'm telling you. If you actually want to improve your knowledge, then you need to think much more deeply about the problems I've pointed out.

 

In a nutshell, this is what your current idea of security looks like: You get messy input, you run it through your semi-magical function which removes all the unwanted and evil parts (code etc.), and out comes a perfectly clean, harmless string which can you can safely include anywhere.

 

This idea is certainly appealing, and it's propagated by hundreds of PHP tutorials. But it's hopelessly naive. It's not just ineffective but actually harmful. The real world doesn't work like this:

• What the user sends you is text. Text by itself doesn't do anything. It's neither “dangerous” nor “unclean”. If certain text triggers security vulnerabilities in your software, that's a defect of your software which you need to fix. Trying to shift the responsibility for this problem on the user by coming up with all kinds of silly input restrictions is fundamentally wrong. There was actually an online banking software which wouldn't even allow you to enter words like “select” or “update” (since those happen to be SQL keywords). Do you really believe this demonstrates their brilliant security concept? Or shouldn't you be worried that they cannot even handle a fucking English word?
• As I've already tried to explain to you, the meaning of input is dependent on the specific language context. The exact same string can have entirely different effects depending on where exactly you insert it. Your simplistic one-size-fits-all function is by definition unable to handle that, because a) it doesn't know the specific context in advance and b) it cannot react to context changes. HTML alone has dozens of different sub-contexts with different semantics. On top of that, you're dealing with mail headers which again have their own syntax rules. And if your software becomes bigger than a single script, you'll be faced with an entire zoo of languages like SQL, XML, shells etc. This is why escaping is an output concept. You apply it right before you insert the value, because only then do you know the actual context.

Understanding this is crucial.

 

When you apply the concepts I've explained to your specific case, you'll end up with something like this:

• If you want to validate certain fields like the e-mail address, you can do that, but this is not a security measure. It will not protect you from anything. It's a usability feature to help the user correct obvious mistakes. This also means you need to get rid of excessive restrictions.
• To handle the mail-related stuff, use a high-level library like PHPMailer. Do not try to assemble the headers yourself, because this will very likely go wrong.
• If you send a plaintext mail, you don't have to escape anything. If you want HTML mails, then you need to escape each inserted value for its specific context. A template engine like Twig can help you with that and will greatly improve the code, because you won't have that PHPHTML mixture any longer.

 

 

 

With regards to stripslashes() [...]

 

For the love of god, strike this function from your memory. Forget that it exists.

 

This function should never have been created, and you obviously don't even understand what it does.

 

 

 

Again I get what you are saying but if you aren’t expecting code in your emails then removing html and php tags is appropriate as it prevents the execution of any code contained within those tags.

 

How on earth is “I <3 PHP” code? This is a heart emoticon. I – love – PHP.

 

Are you telling me that writing this down is a malicious activity which I should no longer be allowed to do?

Edited July 27, 2017 by Jacques1