Xss issue help needed

[zimmo]

Hi,

 

I have a problem where a client has used some software to perform a scan on the site and noticed that the site is vulnerable to xss attacks through the form I show below. Now, I have been told of a function which is also below but I do not have a clue how to implement this to stop the site being xss vulnerable. Can you tell me if the function does not work here? what should I do, as I need to perform something to stop this form and many others on the site being prone to attacks.

 

Please anyway, can you help. I just need to know how this is done... change my code how you feel.

 

 

<?
//# Include the connections script to make a database connection.

include("inc/connect.inc");

//# The form should post to itself.
if ( $_POST['submit'] ) {


$valid = 1;

//# The fields all follow this patern.
//# If you do not require an error check for a field then just use the
//# post field method and not the error check method

$Name = $_POST['Name'];
if ( empty($Name) ) {
$valid = 0;
$Name_error = '<b><font face="Tahoma" color="#FF0000" size=4><span class="style57">Please Enter your Name</span></font></b>';
}

$Company_Name = $_POST['Company_Name'];
if ( empty($Company_Name) ) {
$valid = 0;
$Company_Name_error = '<b><font face="Tahoma" color="#FF0000" size=4><span class="style57">Please Enter your Company Name</span></font></b>';
}

$Telephone = $_POST['Telephone'];
if ( empty($Telephone) ) {
$valid = 0;
$Telephone_error = '<b><font face="Tahoma" color="#FF0000" size=4><span class="style57">Please Enter your Telephone Number</span></font></b>';
}

$Email = $_POST['Email'];
$Web_Site_Comments = $_POST['Web_Site_Comments'];

$antispambox = $_POST['antispambox'];
if ($antispambox == '73634') {}
else
{
$valid = 0;
$antispambox_error = '<b><font face="Tahoma" color="#FF0000" size=4><span class="style57">Please Enter the Numbers as shown in bold</span></font></b>';
}

// End of error checking.

if ( $valid == 1 ) 
{
      // In testing, if you get an Bad referer error
      // comment out or remove the next three lines
      if (strpos($_SERVER['HTTP_REFERER'], $_SERVER['HTTP_HOST'])>7 ||
         !strpos($_SERVER['HTTP_REFERER'], $_SERVER['HTTP_HOST']))
         die("Bad referer");
      $msg="This is the information received from the Spectroscopy Enquiry Form.:\n\n";
      foreach($_POST as $key => $val){
         if (is_array($val)){
            $msg.="Item: $key\n";
            foreach($val as $v){
               $v = stripslashes($v);
               $msg.="***$v\n";
            }
         } else {
            $val = stripslashes($val);
            $msg.="$key: $val\n";
         }
      }
      $recipient="*****";
      $subject="Enquiry Form";
      error_reporting(0);
      if (mail($recipient, $subject, $msg)){
         echo nl2br($input);
      } else
         echo "An error occurred and the message could not be sent.";

header("Location: thanks.php");
exit;
}
}
?>

 

And here is the function. Now can someone tell me if we have to run this function on every field in the form???

 

function RemoveXSS($val) { 
// remove all non-printable characters. CR(0a) and LF(0b) and TAB(9) are allowed 
// this prevents some character re-spacing such as <java\0script> 
// note that you have to handle splits with \n, \r, and \t later since they *are* allowed in some inputs 
$val = preg_replace('/([\x00-\x08][\x0b-\x0c][\x0e-\x20])/', '', $val); 

// straight replacements, the user should never need these since they're normal characters 
// this prevents like <IMG SRC=&#X40&#X61&#X76&#X61&#X73&#X63&#X72&#X69&#X70&#X74&#X3A&#X61&#X6C&#X65&#X72&#X74&#X28&#X27&#X58&#X53&#X53&#X27&#X29> 
$search = 'abcdefghijklmnopqrstuvwxyz'; 
$search .= 'ABCDEFGHIJKLMNOPQRSTUVWXYZ'; 
$search .= '1234567890!@#$%^&*()'; 
$search .= '~`";:?+/={}[]-_|\'\\'; 
for ($i = 0; $i < strlen($search); $i++) { 
	// ;? matches the ;, which is optional 
	// 0{0,7} matches any padded zeros, which are optional and go up to 8 chars 

	// &#x0040 @ search for the hex values 
	$val = preg_replace('/(&#[x|X]0{0,8}'.dechex(ord($search[$i])).';?)/i', $search[$i], $val); // with a ; 
	// &#00064 @ 0{0,7} matches '0' zero to seven times 
	$val = preg_replace('/(&#0{0,8}'.ord($search[$i]).';?)/', $search[$i], $val); // with a ; 
} 

// now the only remaining whitespace attacks are \t, \n, and \r 
$ra1 = Array('javascript', 'vbscript', 'expression', 'applet', 'meta', 'xml', 'blink', 'link', 'style', 'script', 'embed', 'object', 'iframe', 'frame', 'frameset', 'ilayer', 'layer', 'bgsound', 'title', 'base'); 
$ra2 = Array('onabort', 'onactivate', 'onafterprint', 'onafterupdate', 'onbeforeactivate', 'onbeforecopy', 'onbeforecut', 'onbeforedeactivate', 'onbeforeeditfocus', 'onbeforepaste', 'onbeforeprint', 'onbeforeunload', 'onbeforeupdate', 'onblur', 'onbounce', 'oncellchange', 'onchange', 'onclick', 'oncontextmenu', 'oncontrolselect', 'oncopy', 'oncut', 'ondataavailable', 'ondatasetchanged', 'ondatasetcomplete', 'ondblclick', 'ondeactivate', 'ondrag', 'ondragend', 'ondragenter', 'ondragleave', 'ondragover', 'ondragstart', 'ondrop', 'onerror', 'onerrorupdate', 'onfilterchange', 'onfinish', 'onfocus', 'onfocusin', 'onfocusout', 'onhelp', 'onkeydown', 'onkeypress', 'onkeyup', 'onlayoutcomplete', 'onload', 'onlosecapture', 'onmousedown', 'onmouseenter', 'onmouseleave', 'onmousemove', 'onmouseout', 'onmouseover', 'onmouseup', 'onmousewheel', 'onmove', 'onmoveend', 'onmovestart', 'onpaste', 'onpropertychange', 'onreadystatechange', 'onreset', 'onresize', 'onresizeend', 'onresizestart', 'onrowenter', 'onrowexit', 'onrowsdelete', 'onrowsinserted', 'onscroll', 'onselect', 'onselectionchange', 'onselectstart', 'onstart', 'onstop', 'onsubmit', 'onunload'); 
$ra = array_merge($ra1, $ra2); 

$found = true; // keep replacing as long as the previous round replaced something 
while ($found == true) { 
	$val_before = $val; 
	for ($i = 0; $i < sizeof($ra); $i++) { 
		$pattern = '/'; 
		for ($j = 0; $j < strlen($ra[$i]); $j++) { 
			if ($j > 0) { 
				$pattern .= '('; 
				$pattern .= '(&#[x|X]0{0,8}([9][a][b]);?)?'; 
				$pattern .= '|(&#0{0,8}([9][10][13]);?)?'; 
				$pattern .= ')?'; 
			} 
			$pattern .= $ra[$i][$j]; 
		} 
		$pattern .= '/i'; 
		$replacement = substr($ra[$i], 0, 2).'<x>'.substr($ra[$i], 2); // add in <> to nerf the tag 
		$val = preg_replace($pattern, $replacement, $val); // filter out the hex tags 
		if ($val_before == $val) { 
			// no replacements were made, so exit the loop 
			$found = false; 
		} 
	} 
} 
return $val; 
}

[cooldude832]

your function really isn't that great because it appears to be resetting the same valid variable. The Refering thing is also not going to do anything because you can modify headers very easily.  The best way to prevent intrusions is to use a hidden captcha, basically you use the captcha software to set a session that is unique to that form's ID so that it has to come from that forms source.  Also your function for testing for errors only test for empty boxes not the validity of those empty boxes. 

[zimmo]

Thanks for the reply.

 

The form was only set up to make sure they actually enter info, I know it is not a detailed error checker... its just so the user inputs the data... (can you point me in the right direction here for error checking individual fields).

 

The function I took from a website that someone designed for XSS problems... I am unsure how to implement this.

 

It was only today we were told that the forms are open to XSS attacks and we had to fix them, and this is beyond our scope to be honest.

 

Any help is appreciated.

Thanks

barry

[cooldude832]

If making it is out of your scope, i'd suggest you learn some more php (especially if you have scripts that people are testing for security), or find a freelancer to do it for you

[zimmo]

If making it is out of your scope, i'd suggest you learn some more php (especially if you have scripts that people are testing for security), or find a freelancer to do it for you

 

As you know php is an ongoing learning curve... I feel I have learnt alot over the years to get by, being on my own learning does not give me the advanatage of finding all the things we need to know, such as security holes.. but then after looking across the internet, not many people have fixed the holes they have.. just that I have a client who found them..

 

Which is good, but then these (forums) are the places you get help from... you may know alot and yes, I would love to get a freelancer in... if I could afford to...

 

I have always found the forums to be a great source of help...

 

Thanks to those who have pointed me in some direction.. I will look elsewhere to see if I can fix this..

[BradGushurst]

The following code should work, I dont have the time to test it all so any errors are your problem. The function you have is basically a heaping mass of crap and can be done much more efficiently in your case. I would recommend learning how to use Regular Expressions in order to filter your data more effectively. Also I used typecasting to filter one of the variables, which means you need PHP 5 running on the server. Take a look at the comments I left in the code, other things on your server need to be fixed, namely the connect.inc file.

 

Any questions?

 

<?php

// Include the connections script to make a database connection.
include('inc/connect.inc'); /* Should be changed to connect.php so database info cant be revealed */

// The form should post to itself.
if ( isset($_POST['submit']) ) 
{
	$valid = 1;

	/* Good practice to gather all your variables at once and clean them before use */
	$name 		= preg_replace( '/[^a-zA-Z0-9 \.\-]/', '', $_POST['name']); 		/* Any character not specificaly specified in regex will be removed */
	$company 	= preg_replace( '/[^a-zA-Z0-9 .-]/', '', $_POST['company_name']);
	$telephone 	= preg_replace( '/[^0-9\.\-\(\)]/', '', $_POST['telephone']);
	$email		= preg_replace( '/[^a-zA-Z0-9\@]/', '', $_POST['email']); 			/* There are much better algorithms for email regex, google it */
	$antispam	= (int) $_POST['antispambox']; 										/* Use type casting for fields that need to be numbers */

	// Setup error checking variable
	$error = '';

	if( !isset( $name ) || $name == '' ) {
		$error .= 'Please Enter Your Name <br />';
	}

	if( !isset( $company ) || $company == '' ) {
		$error .= 'Please Enter Your Company Name <br />';
	}

	if( !isset( $telephone ) || $telephone == '' ) {
		$error .= 'Please Enter Your Telephone Name <br />';
	}

	if( !isset( $email ) || $email == '' ) {
		$error .= 'Please Enter Your Email Address<br />';
	}

	if( !isset( $antispam ) || $antispam != '73634' ) {
		$error .= 'Please Check Your Anti Spam Entry';
	}


	if( $error == '' ) 
	{
		// In testing, if you get an Bad referer error
		// comment out or remove the next three lines
		if( strpos($_SERVER['HTTP_REFERER'], $_SERVER['HTTP_HOST']) > 7 || !strpos($_SERVER['HTTP_REFERER'], $_SERVER['HTTP_HOST']) )
		{
			die("Bad referer");
		}

		$emailMessage = 'Type this out manually so your dont get unwanted code injected into email, user the variables from above.';

		$recipient = 'user@test.com';
		$subject = 'Enquiry Form';

		error_reporting(0); /* Is there a reason for this? Try not to user this if at all possible */

		if( mail($recipient, $subject, $emailMessage, 'From: website@domain.com') )
		{
			echo nl2br($input); /* What was this for? */
			header('Location: thanks.php');
		} else {
			echo 'An error when attempting to submit your form, please try again later. We are sorry for any inconvenience';/*Dont let people know information is being emailed */
		}
	}
}

// xHTML Form code should follow
if( $error != '' ) { echo '<div class="error">' . $error . '</div>'; }
?>

[cooldude832]

The issue with making forms easier to manage is all in creation, processing, and storing.  The trick to making it work is to use the identical field names for each.  This saves you a lot of headaches because you will never by typing out a string it will all be generated dynamically.

So what you can do is something like

<?php
$error = array();
$fields = array();
foreach($_POST as $key => $value){
$fields[$key] = $value;
if(empty($value)){
$error[$key][] = "Was left Blank";
}
//Do more specific error testing/cleansing on the $field array
if(empty($error)){
//Process query using a foreach loop to write query string
}
else{
echo "<ul>";
foreach($error as $key => $value){
echo "<li>".$key;
echo "<ol>";
foreach($value as $value2){
echo "<li>".$value2."</li>";
}
echo </ol></li>";
}
?>

 

That will display all your errors for each item in a nice list and so forth.  I find this method simply to use because you can just tact on field specific errors, such as telephone # check, email check, is_numeric and so forth