4

Is the following PHP code XSS proof?

<?php

$site = $_GET["url"];

echo "<a href='".str_replace(array("\"", "'", "javascript:", "vbscript:"), array("%22", "%27", "", ""), $site)."'>Site</a>";

?>
4
  • You should use the php function htmlentities() to prevent XSS.
    – ßaron
    Commented Nov 27, 2014 at 16:31
  • Your method does not work in this case. Be aware!
    – Lucas NN
    Commented Nov 27, 2014 at 23:55
  • 1
    Writing filters like this is pretty dangerous, because there are a lot of gotcha cases. Find a good HTML escaping library and use it. Commented Dec 3, 2014 at 2:30
  • @BaileyS Now I can see =)
    – user61429
    Commented Dec 3, 2014 at 2:31

3 Answers 3

6
+100

No, it is not.

Try insert javascript&colon;alert(9) as the URL, or like @wireghoul pointed, JAVASCRIPT:alert(9). Even with javascript :alert(9) in older browsers (IE < 8, I guess).

Better solution:

<?php

header('Content-Type: text/html; charset=utf-8');
header('X-Content-Type-Options: nosniff');

$site = $_GET["url"];
$site_lowercase = strtolower($site);

if(strpos($site_lowercase, "http://")===0 || strpos($site_lowercase, "https://")===0){
    $possible_xss = false;
}
else if(strpos($site_lowercase, "ftp://")===0 || strpos($site_lowercase, "ftps://")===0){
    $possible_xss = false;
}
else if(strpos($site, "/")===0){
    $possible_xss = false;
}
else{
    $possible_xss = true;
}

$site = str_replace(array("\"", "'"), array("%22", "%27"), $site);

echo "<a href='".(($possible_xss)?"//".$site:$site)."'>Site</a>";

?>
10
  • @Rook Your payloads do not work, because my code changes ' for %27. Are you sure? Please try it in writecodeonline.com.
    – Lucas NN
    Commented Nov 27, 2014 at 4:21
  • @Rook I tested my code a lot, and I can not see how it is exploitable.
    – Lucas NN
    Commented Nov 27, 2014 at 4:25
  • 1
    I ran it on writecodeonline.com using $site="JAVASCRIPT:alert(1)";. I would recommend using a whitelist to ensure no malicious data is included in the output.
    – wireghoul
    Commented Nov 27, 2014 at 6:46
  • @wireghoul Your payload does not works in my code.
    – Lucas NN
    Commented Nov 27, 2014 at 6:54
  • 2
    URLs are case sensitive.
    – Gumbo
    Commented Nov 27, 2014 at 7:09
17

I'm generally a fan of not re inventing the wheel because people way smarter than us already did. I did a quick search for you and found the current library you can use: https://code.google.com/p/php-antixss/

Re: your code, it looks a little too simple to prevent fully. I would use a standardized solution that is used by others and continuously contributed to as new exploits develop.

Hope it helps.

2
  • 1
    "people way smarter than us" how do you know? Commented Nov 27, 2014 at 20:42
  • 3
    I would probably say people who put time and effort into this matter already instead.
    – njzk2
    Commented Nov 27, 2014 at 20:55
1

There's a ton of ink written on the subject of why you shouldn't try doing your own homebrew filters for these things! Please have a look at OWASP's XSS mitigation guidelines.

Use the proper libraries for these things - OWASP ESAPI for PHP is a great tool to start with. I'd look at HTML Purifier as well.

You must log in to answer this question.