Edit report at http://bugs.php.net/bug.php?id=54600&edit=1
ID: 54600 Updated by: il...@php.net Reported by: tyra3l at gmail dot com Summary: possible parse_url problem -Status: Open +Status: Bogus Type: Bug Package: *URL Functions PHP Version: 5.3.6 Block user comment: N Private report: N New Comment: Thank you for taking the time to write to us, but this is not a bug. Please double-check the documentation available at http://www.php.net/manual/ and the instructions on how to report a bug at http://bugs.php.net/how-to-report.php Previous Comments: ------------------------------------------------------------------------ [2011-04-25 11:19:09] tyra3l at gmail dot com Description: ------------ I've just read this article: http://theharmonyguy.com/2011/04/21/recent-facebook-xss-attacks-show-increasing- sophistication/ there is an interesting part: "According to Facebook, it turned out that some older code was using PHPâs built-in parse_url function to determine allowable URLs. For example, while parse_url(âjavascript:alert(1)â) yields a scheme of âjavascriptâ and a path of âalert(1)â, adding whitespace gives a different result: parse_url(â javascript:alert(1)â) does not return a scheme and has a path of âjavascript:alert(1)â. Other PHP developers should take note of the difference if parse_url is being used in security-related code." I know that the documentation mentions that "This function is not meant to validate the given URL, it only breaks it up into the above listed parts." but maybe we should do something to prevent people to misuse this function. I see 4 option: - we should improve the code to strip whitespaces that would cause the function to return the same output for the forged url's - we should change te code that the function never parse javascript: as scheme, this would prevent the people to use parse_url for this purpose, but judging from the article at least some code on facebook would fail insecurely for this change. - we should add more documentation about this issue, it can help, but I don't think that this would be the best fix. - leave it as is, we documented that one should not use this for validation, we can't save people from their bad code. Personally I'm not supporting this option. What do you think? Test script: --------------- php -r 'var_dump(parse_url("javascript:alert(1)"));' array(2) { ["scheme"]=> string(10) "javascript" ["path"]=> string(8) "alert(1)" } php -r 'var_dump(parse_url(" javascript:alert(1)"));' array(1) { ["path"]=> string(20) " javascript:alert(1)" } ------------------------------------------------------------------------ -- Edit this bug report at http://bugs.php.net/bug.php?id=54600&edit=1
