PHP form sanitization help

[codernoob]

Hi,

 

I am completely new to PHP.. and have been trying to build a webform for a website.

 

I have set the form using HTML/CSS right and have added a JS script to validate it on the client side (it basically throws an error if the input isnt correct) on the website.

 

But the issue is my PHP back-end form processor aint working right .. I think am getting XSS attacks .. and would like some help to edit my current PHP form.

 

Can someone help me with it? The pHp code is below:

 

<?php
$title = $_POST['title'];
$name = $_POST['name'];
$email = $_POST['email'];
$phone = $_POST['phone'];
$designation = $_POST ['jobt'];
$company = $_POST['cname'];
$event = $_POST['event'];
$purpose = $_POST['purpose'];
$message = $_POST['message'];
$TandC = $_POST['TandC'];
$formcontent=" From: \n Name: $title $name \n Email: $email \n Phone: $phone \n Designation: $designation \n Company: $company \n event: $event  \n Interested in being a: $purpose \n Message: $message";
$recipient = "brochure@xyz.com";
$subject = "Brochure Request";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
if (isset($_POST['email'])) {
header("Location:thanks.html");
}
?>

Edited January 5, 2013 by codernoob

[Pikachu2000]

Javascript is not validation. Validation must be done server-side to be effective.

 

Now, what makes you think you're getting XSS attacks? What is happening/not happening that should not/should happen?

[Andy123]

Javascript validation is OK as long as you back it up with server side validation as well. Otherwise it's easy to simply turn of Javascript in the browser. To avoid XSS attacks, use the htmlentities function whenever you are printing user supplied data on your pages.

[Pikachu2000]

That's exactly my point. If it can be disabled by the user, it isn't validation at all.

[codernoob]

Javascript is not validation. Validation must be done server-side to be effective.

 

Now, what makes you think you're getting XSS attacks? What is happening/not happening that should not/should happen?

 

Thanks for the response..

 

I get mails with empty fields in the mail set for the form + there are certain mails which have <script><xssv...></script>. Although that could be sitelock's way of telling me about the issue.

 

But is it possible to have the validation done on this form (the code above) without throwing an error to the page? I was hoping to keep the webpage HTML/CSS form separate from my backend PHP form processor.

 

 

 

 

Javascript validation is OK as long as you back it up with server side validation as well. Otherwise it's easy to simply turn of Javascript in the browser. To avoid XSS attacks, use the htmlentities function whenever you are printing user supplied data on your pages.

 

Thanks. Does it mean that I need to use function on the backend PHP form?

[Andy123]

I get mails with empty fields in the mail set for the form + there are certain mails which have <script><xssv...></script>. Although that could be sitelock's way of telling me about the issue.

 

That is probably the result of spam robots filling out and submitting your form. These robots do not execute Javascript code.

 

But is it possible to have the validation done on this form (the code above) without throwing an error to the page? I was hoping to keep the webpage HTML/CSS form separate from my backend PHP form processor.

 

What do you mean by "without throwing an error to the page"? You would probably want to tell the user what went wrong instead of doing nothing, even though 99%+ of the users do have Javascript enabled. That is up to you, though. It is quite easy, so I would recommend it. I do not know if your HTML form is in the same file as your PHP code, but I will assume this for simplicity.

 

$errors = array();

$title = $_POST['title'];

if (empty($title)) {
$errors[] = 'You must enter a title';
}

// Validate other fields here

if (empty($errors)) {
// No errors; send e-mail here and redirect
}

// Show errors
else {
echo '<ul class="errors">';

foreach ($errors as $error) {
echo '<li>' . $error . '</li>';
}

echo '</ul>';
}

 

Thanks. Does it mean that I need to use function on the backend PHP form?

 

Whenever you print data on your pages that comes from your users, you will want to use that function. For instance, let's say that you want to show a confirmation of the data that the user submitted. You could do it like this:

 

<h1>Thank you for your submission</h1>

<p>Here is the data that you submitted:</p>

<?php

echo 'Title: ' . htmlentities($_POST['title']) . '<br />';
echo 'Name: ' . htmlentities($_POST['name']) . '<br />';
echo 'E-mail: ' . htmlentities($_POST['email']) . '<br />';

?>

 

If someone enters <script type="text/javascript">alert("Hello World!");</script> as the title, this code will not be executed by the browser because the illegal signs are transformed to the appropriate HTML entities. This basically means that the characters will be displayed as they were entered, but the browser will not execute the code (or render any markup entered).

[codernoob]

 

What do you mean by "without throwing an error to the page"? You would probably want to tell the user what went wrong instead of doing nothing, even though 99%+ of the users do have Javascript enabled. That is up to you, though. It is quite easy, so I would recommend it. I do not know if your HTML form is in the same file as your PHP code, but I will assume this for simplicity.

 

$errors = array();

$title = $_POST['title'];

if (empty($title)) {
$errors[] = 'You must enter a title';
}

// Validate other fields here

if (empty($errors)) {
// No errors; send e-mail here and redirect
}

// Show errors
else {
echo '<ul class="errors">';

foreach ($errors as $error) {
echo '<li>' . $error . '</li>';
}

echo '</ul>';
}

 

 

Thanks Andy, I actually have the PHP form processor in a separate file which is called by form action.

 

there is a separate JS script that does the check and throws errors (hopefully 99% of users have js enabled). Honestly am a tad unsure about using php form within an html page (one reason I am using js to do the client side validation) - you can check it working here : http://quadimensionevents.com/contacts.html

 

So using your logic, is it fair to say that, the below edited form would work?

 

<?php


$name = htmlentities (isset($_POST['name']));
$email = htmlentities (isset($_POST['email']));
$phone = htmlentities (isset($_POST['phone']));
$message = htmlentities (isset($_POST['message']));
$formcontent=" From: $name \n Email: $email \n Phone: $phone \n Message: $message";
$recipient = "queries@xxxx.com";
$subject = "Contact Form";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
if (isset($_POST['email'])) {
header("Location: thanks.html");
}


?>

 

Apologies if this is way too stupid, I checked around on the web but couldnt really get a tut that really explains the basics..

 

thanks again for all the help

[haku]

That's exactly my point. If it can be disabled by the user, it isn't validation at all.

 

Sorry to be picky, but it most definitely is validation - it validates whether or not the input is correct. It's just not an effective validation method when used alone as it can be bypassed.

[Pikachu2000]

Again, javascript is not validation. You must do validation on the server side. All javascript can be considered is a convenience to the user when properly implemented, or possibly a large annoyance when improperly implemented. Using htmlentities is fine to help prevent XSS, but you also need to add code server side to prevent submitting the form with required fields left empty and prevent email header injection. The problem is almost never caused by human users, but by spambots, and they don't pay attention to javascript at all.

[haku]

Honestly am a tad unsure about using php form within an html page (one reason I am using js to do the client side validation)

Forms require two parts to function:

1) The front end HTML

2) The back end process

 

All web forms are built in HTML (part 1). But this HTML alone is will not do anything without a back-end script to process the form submission. This back-end can be done using any server scripting language, but since this is a PHP forum, of course we will talk about PHP here. So while you may be unsure about using PHP, you will need to use some sort of backend process on your form, or else your form will not do anything.

 

On top of form submission in the backend (which I'll refer to as PHP from here on), you will want to validate the user supplied data, or else you can run into problems, as is being discussed on this page. Javascript validation can also be used, but since users can easily disable javascript, you will also want to include a process on the backend that validates the submitted data. If you do not do this, and a user disables javascript, validation is not performed, and your form becomes vulnerable.

 

So using your logic, is it fair to say that, the below edited form would work?

 

<?php
$name = htmlentities (isset($_POST['name']));
$email = htmlentities (isset($_POST['email']));
$phone = htmlentities (isset($_POST['phone']));
$message = htmlentities (isset($_POST['message']));
$formcontent=" From: $name \n Email: $email \n Phone: $phone \n Message: $message";
$recipient = "queries@xxxx.com";
$subject = "Contact Form";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader) or die("Error!");
if (isset($_POST['email'])) {
header("Location: thanks.html");
}


?>

htmlentities() is used when printing text to a browser. What happens is that a browser will parse text for code - HTML, javascript and CSS. If it finds this code, then it executes/renders it. So if the user has submitted javascript, and that javascript is sebt to the browser, the javascript is executed rather than being printed. This is bad. Using htmlentities() converts certain characters to an 'HTML entity'. An HTML entity is a series of characters that shows up as a different character in the browser. An example is the less than symbol (<). The HTML entity for this is

<

. If you look at the HTML source of the page, you will see the characters I just showed, but when viewing the page itself, you will see the less than symbol.

 

The relevance to this is that your code is for sending emails - which aren't a browser. So if you use htmlentities() on this code, it will convert all the symbols to entities, and make the email essentially unreadable to human eyes, since the user will be shown the HTML entities, rather than the symbol they represent.

 

The point of this being that you shouldn't use htmlentities() for emails.

 

Note: You may be thinking 'but I view my emails in a browser' (eg. gmail). In such a case, the application that is showing you the email will almost definitely already be using some function that converts any symbols to their HTML entities, so you don't need to do it on your end.

[haku]

Again, javascript is not validation.

And again, yes it is. It's just not effective validation for security purposes.

 

If my site requires that usernames not contain any symbols, and the user enters a $ symbol as part of their username, I can use javascript to tell the user that this symbol is not valid - aka validation. Of course, this can be bypassed if the user does not have javascript enabled, but that does not change the fact that the javascript is validating the input.

Edited January 6, 2013 by haku

[Pikachu2000]

My response wasn't aimed at you; I must have had the thread open before you submitted. But honestly, as far as I'm concerned since it can't, with certainty, prevent things from being submitted that shouldn't be submitted, it can't be considered validation. So I guess we'll have to agree to disagree on that point.

 

Anyhow, I haven't seen you around in a while. Good to see you back.

[codernoob]

Forms require two parts to function:

1) The front end HTML

2) The back end process

 

All web forms are built in HTML (part 1). But this HTML alone is will not do anything without a back-end script to process the form submission. This back-end can be done using any server scripting language, but since this is a PHP forum, of course we will talk about PHP here. So while you may be unsure about using PHP, you will need to use some sort of backend process on your form, or else your form will not do anything.

 

 

 

Thanks again.

 

Actually I have both 1 and 2.

 

The front end html is fine and I have added JS validation to it - which I know is not fool proof. But thats fine if i have part two right.

 

The part 2 - is where am having the problem. The PHP code i posted is my back-end processor. But I havent been able to set it up right to sanitize my data.

 

Do you think, adding this will work? - filter_var($value, FILTER_SANITIZE_NUMBER_INT); to all my inputs on my PHP form?

 

Sorry, may be am doing it all wrong and getting your inputs wrong!

[codernoob]

Ok , i did some looking around and made some modifications,

 

Can you check and let me know if my sanitization is good enough for the back end php?

 

<?php

$title = $_POST['title'];
$name = $_POST['name'];
$email = $_POST['email'];
$phone = $_POST['phone'];
$message = $_POST['message'];
$recipient = "queries@xyz.com";
$subject = "contact";


$dodgy_strings = array(
               "content-type:"
               ,"mime-version:"
               ,"multipart/mixed"
               ,"bcc:"
);


function is_valid_email($email) {
 return preg_match('#^[a-z0-9.!\#$%&\'*+-/=?^_`{|}~]+@([0-9.]+|([^\s]+\.+[a-z]{2,6}))$#si', $email);
}


function contains_bad_str($str_to_test) {
 $bad_strings = array(
               "content-type:"
               ,"mime-version:"
               ,"multipart/mixed"
 ,"Content-Transfer-Encoding:"
               ,"bcc:"
 ,"cc:"
 ,"to:"
 );

 foreach($bad_strings as $bad_string) {
   if(eregi($bad_string, strtolower($str_to_test))) {
     echo "$bad_string found. Suspected injection attempt - mail not being sent.";
     exit;
   }
 }
}


function contains_newlines($str_to_test) {
  if(preg_match("/(%0A|%0D|\\n+|\\r+)/i", $str_to_test) != 0) {
    echo "newline found in $str_to_test. Suspected injection attempt - mail not being sent.";
    exit;
  }
} 


if($_SERVER['REQUEST_METHOD'] != "POST"){
  echo("Unauthorized attempt to access page.");
  exit;
}


if (!is_valid_email($email)) {
 echo 'Invalid email submitted - mail not being sent.';
 exit;
}


contains_bad_str($email);
contains_bad_str($name);
contains_bad_str($phone);
contains_bad_str($message);


contains_newlines($email);
contains_newlines($subject);


$formcontent=" From: \n $title $name \n Email: $email \n Phone: $phone \n Message: $message";
$mailheader = "From: $email \r\n";
mail($recipient, $subject, $formcontent, $mailheader);


if (isset($_POST['email'])) 
{
header("Location: thanks.html");}
?>;

[codernoob]

can someone confirm if this is correct??

 

many thanks!

[codernoob]

its ok. I fixed it myself thanks.

 

If anyone wants a form script, just let know.

 

Thanks!

[Christian F.]

Quick list of the issues I've found:

• You're not checking to see if the $_POST array contains any submitted content, before you try to use it. Which will cause notices about "undefined index" to be thrown.
  
• Associated with that, you're not doing any kind of validation, to ensure that the user has actually posted (legit) content.
  
• The $dodgy_strings variable is misplaced, and not used anyway.
  
• The REQUEST_METHOD check should be the first thing in this script, as you wouldn't want to execute any of the code without a form being submitted to it first.
  
• Instead of the is_valid_email () function you should be using filter_val () with the VALIDATE_EMAIL flag.
  
• There is no point in checking for MAIL header strings in content that goes into the mail body, nor multipart headers for a non-multipart e-mail.
  
• The e-mail will not be allowed to contain newlines by virtue of the e-mail validation in the first place, and the subject is a static string set by yourself. Thus checking for newlines in these variables is a complete waste.
  
• Mail headers should be terminated by [ci]\r\n[/ic], and not just \n alone. I would recommend extending this to the mail body as well, just to ensure that you're not going to have issues with certain e-mail clients. Also there shouldn't be any other whitespace before or after the newlines, especially not in the headers.
  
• You're not checking whether or not mail() returned an error, thus you have no way of knowing whether or not the mail was actually sent to the MTA.
  
• Checking if $_POST['email'] isset at the very end is completely backwards, and by that point quite useless. The script has gone through all of the other code, assuming the fields have been posted, so why only check for it before trying to redirect the user to the confirmation screen?
  Check for submission as before you do the validation, at the very top of the script.
  
• You're lacking a call to die () after the redirect. Granted, probably not an issue here as you seem to end the script on the redirect anyway. However, if this is an included file, or you had more content below the redirect, you would need to kill the script. Otherwise the parser would continue to execute the PHP code, possibly causing major issues.

 

All in all I think you've done what a lot of people unfortunately do: Searched online for a tutorial, found something way out of date and written by someone who really didn't know what they were doing, and commented on by people who knew even less and thought it was great.

Not your fault, but to rectify this you need to read up on proper input validation, how e-mail headers are constructed, and how to make a proper and secure mailing script. I do recommend using the PHPmailer class for this, as it'll help alleviate a lot of the work you need to do.