Page 1 of 1

Re: XSS prevention

Posted: Fri Aug 17, 2007 2:01 am
by kkonline
feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]


Someone suggested of using a token verification in the form
Well i got the following code

Code: Select all

<?php

$token = md5(uniqid(rand(), TRUE));
$_SESSION['token'] = $token;
$_SESSION['token_timestamp'] = time();

?>

<form action="/post.php" method="POST">
<input type="hidden" name="token" value="<?php echo $token;?>" />
<p>Subject: <input type="text" name="subject" /></p>
<p>Message: <textarea name="message"></textarea></p>
<p><input type="submit" value="Add Post" /></p>
</form>

Then when you get to your validation page include this.

if ($_POST['token']!= $_SESSION['token']) {
echo "Invalid data!";
exit;
}
$token_age = time() - $_SESSION['token_time'];
if ($token_age >= LOGIN_TIME_LIMIT) {
// time limit can be set here as number instead
// of LOGIN_TIME_LIMIT define, such as 60*10
exit;
}
Is the above code correct?


feyd | Please use

Code: Select all

,

Code: Select all

and [syntax="..."] tags where appropriate when posting code. Your post has been edited to reflect how we'd like it posted. Please read:  [url=http://forums.devnetwork.net/viewtopic.php?t=21171]Posting Code in the Forums[/url] to learn how to do it too.[/color]

Posted: Fri Aug 17, 2007 5:33 am
by Mordred
It seems you are trying to combat XSRF, not XSS.

The code is more or less okay (bar some coding style issues which are irrelevant here), but you must be aware that if there is an XSS hole in your server somewhere, XSRF can still be done by an injected script that fetches the form, parses out the token and then submits the form with the correct token.

It would also work only for POST forms which may or may not be what you intended.

Posted: Fri Aug 17, 2007 6:52 am
by kkonline
Hi am conducting some xss test with the following code:

Code: Select all

function sql_safe($value)
{
 if (get_magic_quotes_gpc())
{
$value=stripslashes($value);
return $value;
}
else
{ 
  $value = trim($value);
  $value = strip_tags($value);
  $value = htmlentities($value);
  $value = mysql_real_escape_string($value);
  return $value;
  // this could all be done on one line, but for simplicity I seperated it all
}
}
Should I use a self written function like

Code: Select all

function clean($i){
if (!get_magic_quotes_gpc()){
$i = addslashes($i);
}
$i = rtrim($i);
$look = array('&', '#', '<', '>', '"', '\'', '(', ')');
$safe = array('&', '&#35', '<', '>', '"', '&#39', '(', ')');
$i = str_replace($look, $safe, $i);
return $i;
}
But it doesn't seem to work FULLY... so any additions or modification to the above code. Or some other suggestions?

Where can i find a better code?

Posted: Fri Aug 17, 2007 7:00 am
by superdezign
Did you just change your post...?

Anyway, that code doesn't do much. The first function either removes slashes OR attempts to clean the data... it looks pointless. The second one seems just.. wasteful.

In all honesty, when cleaning data, all you really need is mysql_real_escape_string for MySQL queries and htmlspecialchars for HTML data. If you are going to print user data (and don't want to allow HTML formatting or you already filter the HTML in the data), use htmlspecialchars(). If you don't use them, then you need to filter more specifically.


And what do you mean by conduction XSS tests? What would that code do at all for XSS tests? What are you testing for, exactly?

Posted: Fri Aug 17, 2007 7:18 am
by Mordred
Both functions are wrong. The first is named as if it would protect against sql injection (it wouldn't), but you test it agains XSS? The second mimics htmlspecialchars(), but is not it.

What are you protecting against? Learn more about the attack itself first. A simple "escape my input" function is sometimes not enough, you may need to take complex measures to avoid injection if you do some stuff with the output.

You can't have a function that protects against everything at the same time.
Read the security sections in the PHP manual AT LEAST, and possibly a book or two on the subject (Chris Shiflett's seems to be the most popular choice).

Good luck :)

Posted: Sun Aug 19, 2007 4:44 am
by kkonline
superdezign wrote: In all honesty, when cleaning data, all you really need is mysql_real_escape_string for MySQL queries and htmlspecialchars for HTML data. If you are going to print user data (and don't want to allow HTML formatting or you already filter the HTML in the data), use htmlspecialchars(). If you don't use them, then you need to filter more specifically.
2> You should never use htmlentities or htmlspecialchars as these are encodings for a specific document format. You could not do searching or matching on that data and displaying it in any other document format would take extra code, and could not be done directly from the database.

In the thread relating SQL Injection Prevention the above point was stated and was agreed with by most of the senior members of the forum

I have done prevention from sql injection and need to prevent the data against xss attacks in this process most of the guides including Shiflett's recommend the use of htmlentities(), striptags()

If i use the htmlentities() on the output from the database and then display. Is it safe enough against xss attacks?

The AIM
I just want to send the data in form field subject, story and titletext without any html tags to database. So i think if i use htmlentities() on the input to db would it make any problem to search? I database i just want SIMPLE TEXT but maybe some ' or " as user will submit stories.

Another thing is that i got the following code as recommended by Shiflett

Code: Select all

<?php

// +----------------------------------------------------------------------+
// | popoon                                                               |
// +----------------------------------------------------------------------+
// | Copyright (c) 2001-2006 Bitflux GmbH                                 |
// +----------------------------------------------------------------------+
// | Licensed under the Apache License, Version 2.0 (the "License");      |
// | you may not use this file except in compliance with the License.     |
// | You may obtain a copy of the License at                              |
// | http://www.apache.org/licenses/LICENSE-2.0                           |
// | Unless required by applicable law or agreed to in writing, software  |
// | distributed under the License is distributed on an "AS IS" BASIS,    |
// | WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or      |
// | implied. See the License for the specific language governing         |
// | permissions and limitations under the License.                       |
// +----------------------------------------------------------------------+
// | Author: Christian Stocker <chregu@bitflux.ch>                        |
// +----------------------------------------------------------------------+
//
// $Id$

class popoon_classes_externalinput {
    
    // this basic clean should clean html code from
    // lot of possible malicious code for Cross Site Scripting
    // use it whereever you get external input    

    static function basicClean($string) {
        if (get_magic_quotes_gpc()) {
            $string = stripslashes($string);
        }
        $string = str_replace(array("&","<",">"),array("&amp;","&lt;","&gt;",),$string);
        // fix &entitiy\n;
        
        $string = preg_replace('#(&\#*\w+)[\x00-\x20]+;#u',"$1;",$string);
        $string = preg_replace('#(&\#x*)([0-9A-F]+);*#iu',"$1$2;",$string);
        $string = html_entity_decode($string, ENT_COMPAT, "UTF-8");
        
        // remove any attribute starting with "on" or xmlns
        $string = preg_replace('#(<[^>]+[\x00-\x20"\'])(on|xmlns)[^>]*>#iUu',"$1>",$string);
        // remove javascript: and vbscript: protocol
        $string = preg_replace('#([a-z]*)[\x00-\x20]*=[\x00-\x20]*([\`\'"]*)[\\x00-\x20]*j'. '[\x00-\x20]*a[\x00-\x20]*v[\x00-\x20]*a[\x00-\x20]*s[\x00-\x20]*c[\x00-\x20]'. '*r[\x00-\x20]*i[\x00-\x20]*p[\x00-\x20]*t[\x00-\x20]*:#iUu','$1=$2nojavascript...',$string);
        $string = preg_replace('#([a-z]*)[\x00-\x20]*=([\'"]*)[\x00-\x20]*v[\x00-\x20]*b'. '[\x00-\x20]*s[\x00-\x20]*c[\x00-\x20]*r[\x00-\x20]*i[\x00-\x20]'. '*p[\x00-\x20]*t[\x00-\x20]*:#iUu','$1=$2novbscript...',$string);
        $string = preg_replace('#([a-z]*)[\x00-\x20]*=([\'"]*)[\x00-\x20]*-moz-binding[\x00-\x20]*:#Uu','$1=$2nomozbinding...',$string);
        //<span style="width: expression(alert('Ping!'));"></span> 
        // only works in ie...
        $string = preg_replace('#(<[^>]+)style[\x00-\x20]*=[\x00-\x20]*'. '([\`\'"]*).*expression[\x00-\x20]*\([^>]*>#iU',"$1>",$string);
        $string = preg_replace('#(<[^>]+)style[\x00-\x20]*=[\x00-\x20]*'. '([\`\'"]*).*behaviour[\x00-\x20]*\([^>]*>#iU',"$1>",$string);
        $string = preg_replace('#(<[^>]+)style[\x00-\x20]*=[\x00-\x20]*'. '([\`\'"]*).*s[\x00-\x20]*c[\x00-\x20]*r[\x00-\x20]*i[\x00-\x20]*'. 'p[\x00-\x20]*t[\x00-\x20]*:*[^>]*>#iUu',"$1>",$string);
        //remove namespaced elements (we do not need them...)
        $string = preg_replace('#</*\w+:\w[^>]*>#i',"",$string);
        //remove really unwanted tags
        
        do {
            $oldstring = $string;
            $string = preg_replace('#</*(applet|meta|xml|blink|link|style|script|embed|object|'. 'iframe|frame|frameset|ilayer|layer|bgsound|title|base)[^>]*>#i',"",$string);
        } while ($oldstring != $string);
        
        return $string;
    }
    
    static function removeMagicQuotes($data) {
        
        if (get_magic_quotes_gpc()) {
            $newdata = array();
            foreach ($data as $name => $value) {
                $name = stripslashes($name);
                if (is_array($value)) {
                    $newdata[$name] = self::removeMagicQuotes($value);
                } else {
                    $newdata[$name] = stripslashes($value);
                }
            }
            return $newdata;
        }
        return $data;
        
    }
}
If i have a variable as $subject=$_POST['subject'] then how do i use the above code to clean the variable. I mean how to call the class written above to return the clean subject?

Posted: Sun Aug 19, 2007 6:41 am
by kkonline
superdezign wrote: In all honesty, when cleaning data, all you really need is mysql_real_escape_string for MySQL queries and htmlspecialchars for HTML data. If you are going to print user data (and don't want to allow HTML formatting or you already filter the HTML in the data), use htmlspecialchars(). If you don't use them, then you need to filter more specifically.
In short I should use mysql_real_escape_string on any form input to the db and htmlentities or/and htmlspecialchars to any output from db which is to be displayed

Am i correct??? This is for prevention from sql injection (mysql_real_escape() and from xss attacks(htmlentities/htmlspecialchars())

Is there anything i need to apply to user data going to db to stop xss attacks?? striptags should be applied to input to db or output from db??

Posted: Sun Aug 19, 2007 7:36 am
by matthijs
In short I should use mysql_real_escape_string on any form input to the db and htmlentities or/and htmlspecialchars to any output from db which is to be displayed
Basically yes.
Is there anything i need to apply to user data going to db to stop xss attacks?? striptags should be applied to input to db or output from db??
What you have to understand is the following: when any data comes into your script, you have to validate/filter the data. So often that's user input, but other times it's data from some other source. Validating/filtering means that you want to make sure the data is in a form/format you expect. Contains the characters you want. To give an example: when you have a form with a field for telephone, you want people to fill in only numbers. Maybe also - and spaces. So you do this as strict as possible, so that any data that is being processed further is "clean".

That was input. Next step is output. Output is data going somewhere. That can be a database. Or output to HTML. or somewhere else. Each kind of output has it's own kind of escaping. For the database you use a db specific escaping function. For mysql that's mysql_real_escape_string. For HTML that's html_entities or htmlspecialchars.

But really, I advice to read more about this. the book by Shifflet, his website with many examples, or other PHP security books are a good start.

Posted: Sun Aug 19, 2007 2:28 pm
by Mordred
kkonline wrote: In short I should use mysql_real_escape_string on any form input to the db and htmlentities or/and htmlspecialchars to any output from db which is to be displayed

Am i correct??? This is for prevention from sql injection (mysql_real_escape() and from xss attacks(htmlentities/htmlspecialchars())
God, NO! You need to understand the attack(s) in full before taking precautions. There is NO panacea function to be applied to a random input that would make it safe (without breaking it; otherwise allowing only the letter 'a' is perfectly safe :) (*) ) against SQL injection or XSS. You need to know the detailed context in which the input is used and act accordingly, and any such knowledge is external to the "cleaning" function. That's why you need to understand the attack before starting to develop protections against it.
kkonline wrote: Is there anything i need to apply to user data going to db to stop xss attacks?? striptags should be applied to input to db or output from db??
XSS happens when you print to the (usually HTML) output. Forget the database, it has no role here. If anything shouldn't contain tags, strip them, it doesn't matter where it comes from. (Bear in mind that striptags is no guarantee that XSS would not yet be possible)
Another thing is that i got the following code as recommended by Shiflett
He recommends this crap? It's [s]totally[/s] useless (correction, it's useless, but at least not totally :)). Where did you read that he recommend this? (If it's true, I should better stop recommending him, it seems)

Edit: Ah, you misunderstood. http://shiflett.org/blog/2005/jan/xss-cheatsheet. He recommends the cheatsheet, not the filter :)


-------
(*) btw allowing only the letter 'a' is still not a good protection, you can still do overflows and whatnot :P