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>";

?>
  • You should use the php function htmlentities() to prevent XSS. – ßaron Nov 27 '14 at 16:31
  • Your method does not work in this case. Be aware! – Lucas NN Nov 27 '14 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. – trognanders Dec 3 '14 at 2:30
  • @BaileyS Now I can see =) – user61429 Dec 3 '14 at 2:31
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>";

?>
  • @Rook Your payloads do not work, because my code changes ' for %27. Are you sure? Please try it in writecodeonline.com. – Lucas NN Nov 27 '14 at 4:21
  • @Rook I tested my code a lot, and I can not see how it is exploitable. – Lucas NN Nov 27 '14 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 Nov 27 '14 at 6:46
  • @wireghoul Your payload does not works in my code. – Lucas NN Nov 27 '14 at 6:54
  • 2
    URLs are case sensitive. – Gumbo Nov 27 '14 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.

  • 1
    "people way smarter than us" how do you know? – user3459110 Nov 27 '14 at 20:42
  • 3
    I would probably say people who put time and effort into this matter already instead. – njzk2 Nov 27 '14 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.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy