PHP Developers Network

A community of PHP developers offering assistance, advice, discussion, and friendship.
 
Loading
It is currently Thu Nov 20, 2014 9:08 pm

All times are UTC - 5 hours




Post new topic Reply to topic  [ 13 posts ] 
Author Message
PostPosted: Fri Jan 27, 2012 12:35 pm 
Offline
Forum Contributor
User avatar

Joined: Sun Nov 13, 2005 8:35 am
Posts: 212
Location: Folkestone, Kent, UK
I have a function that I am using to hopefully make sure that data is safe.

Syntax: [ Download ] [ Hide ]
function xssfilter($data) {
       
        if(!get_magic_quotes_gpc()) {
                $data = addslashes($data);
                    }
                        $data = strip_tags(htmlspecialchars($data));
                       
        $data = mysql_real_escape_string($data);
       
        return $data;
       
}
 

I use it as such

$var = xssfilter($_POST['var']);

Does this look secure enough?


Top
 Profile  
 
PostPosted: Fri Jan 27, 2012 2:04 pm 
Offline
Forum Contributor
User avatar

Joined: Thu May 11, 2006 8:58 pm
Posts: 305
Location: Utah, USA
You are confusing XSS with SQL injection.

The simplest way to avoid XSS is to run htmlspecialchars() before echoing anything. I don't recommend putting anything in the database with htmlspecialchars already run. Wait until output. Some day you may want to send your database data via a web service or print a check. It looks pretty stupid if you print a check made out to "Mr. O'Reilly".

The exception would be rich text. If you are accepting rich text like a blog post, you should run it through a full HTML parser and cleaner such as HTMLPurifier before putting it in the database.

As far as SQL injection, before putting anything into the database always use PDO binding or mysql_real_escape_string(). And addslashes() is unnecessary. In fact, addslashes() is rarely useful.


Top
 Profile  
 
PostPosted: Fri Jan 27, 2012 5:43 pm 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2775
Location: .za
If you are going to use the code, get rid of the magic quotes checks and apply filtering regardless of it; magic_quotes_gpc has been deprecated and shouldn't be relied upon at all.

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Fri Jan 27, 2012 5:45 pm 
Offline
Moderator
User avatar

Joined: Tue Nov 09, 2010 3:39 pm
Posts: 3744
Location: Montreal, Canada
social_experiment wrote:
If you are going to use the code, get rid of the magic quotes checks and apply filtering regardless of it; magic_quotes_gpc has been deprecated and shouldn't be relied upon at all.

Though at that point all you're left with is a mysql_real_escape_string wrapper function.

_________________
Stay on top of upgrades.
Supported PHP versions
No longer supported versions


Top
 Profile  
 
PostPosted: Sat Jan 28, 2012 3:22 am 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2775
Location: .za
I should have probably worded that better :) I mean he can leave the checks for magic_quotes_gpc() out; since it's no longer useful but still keep the other filters in place. This strip_tags(htmlspecialchars($data)); is redundant as htmlspecialchars() will make sure strip_tags() find nothing to strip, i would change it like this:
Syntax: [ Download ] [ Hide ]
<?php
function xssfilter($data) {        
        $data = addslashes($data);
        #
       $data = strip_tags($data);
         #              
       $data = mysql_real_escape_string($data);
        #
       return $data;        
}
?>

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Sun Jan 29, 2012 6:30 am 
Offline
Forum Contributor
User avatar

Joined: Sun Nov 13, 2005 8:35 am
Posts: 212
Location: Folkestone, Kent, UK
Thanks Will do.


Top
 Profile  
 
PostPosted: Mon Jan 30, 2012 10:59 am 
Offline
Forum Contributor
User avatar

Joined: Thu May 11, 2006 8:58 pm
Posts: 305
Location: Utah, USA
@social_experiment, your function probably behaves different than you are thinking. If you pass the string "Jane's <3 is Joe", you will get out of the database "Jane\'s ". Note how strip_tags() cuts off strings with less than signs in the middle. But if you run mysql_real_escape_string() alone on the way in, and htmlspecialchars() before displaying to the browser, the string will look identical.


Top
 Profile  
 
PostPosted: Mon Jan 30, 2012 4:27 pm 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2775
Location: .za
Thanks for pointing that out tr0gd0rr :) I personally don't use strip_tags() [and it shows doesn't it]; if you catch " or ' (which mysql_real_escape_string() does) and use htmlspecialchars() when displaying data then strip_tags() becomes redundant

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Wed Feb 01, 2012 7:50 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
Syntax: [ Download ] [ Hide ]
$data = htmlspecialchars($data, ENT_QUOTES, 'utf8');


None of the other listed options so far will work correctly:
strip_tags() is of dubious value and will damage the original data,
mysql_real_escape_string() is about MySQL and SQL injection, not XSS
addslashes() is just silly
htmlspecialchars($data) is insufficient protection when used in attribute context.


Top
 Profile  
 
PostPosted: Wed Feb 01, 2012 10:26 pm 
Offline
Forum Contributor
User avatar

Joined: Thu May 11, 2006 8:58 pm
Posts: 305
Location: Utah, USA
Mordred wrote:
htmlspecialchars($data) is insufficient protection when used in attribute context.

What do you mean exactly? Because an attribute might contain some JavaScript? Or how is it insufficient?


Top
 Profile  
 
PostPosted: Thu Feb 02, 2012 5:41 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
If it's an attribute and you use the 'wrong' type of quotes, plain htmlspecialchars() will fail to protect against XSS, read the docs. It's amazing how stupid some of the PHP functions are - it's a security measure which is insecure by default!


Top
 Profile  
 
PostPosted: Thu Feb 02, 2012 5:57 am 
Offline
DevNet Master
User avatar

Joined: Sun Feb 15, 2009 12:08 pm
Posts: 2775
Location: .za
So a function such as htmlentities() with ENT_QUOTES and charset should be the default if you wish to properly safeguard against XSS?

_________________
“Don’t worry if it doesn’t work right. If everything did, you’d be out of a job.” - Mosher’s Law of Software Engineering


Top
 Profile  
 
PostPosted: Thu Feb 02, 2012 8:14 am 
Offline
DevNet Resident
User avatar

Joined: Sun Sep 03, 2006 5:19 am
Posts: 1579
Location: Sofia, Bulgaria
Mordred wrote:
Syntax: [ Download ] [ Hide ]
$data = htmlspecialchars($data, ENT_QUOTES, 'utf8');




Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 13 posts ] 

All times are UTC - 5 hours


Who is online

Users browsing this forum: No registered users and 2 guests


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Jump to:  
Powered by phpBB® Forum Software © phpBB Group