r0b Posted May 8, 2011 Share Posted May 8, 2011 I opened a thread yesterday about an XSS vulnerability when the user is logged in. I'll summarize is in a short quote: http://host/editText.php?fieldname=slogan&content=slogan<img src=x onerror=alert("XSS")> This vulnerability only works if the user is logged in. I want to secure it anyway to give the security companies contacting me about this a break. xyph solved my problem with this: foreach( $_REQUEST as $key => $val ) $_REQUEST[$key] = htmlentities($val); He warned me it was a risky but I didn't take him that seriously. Well guess he was right. The foreach loop he gave me does protect me from the XSS attack, but it also disables the users to use any kind of code in the pages. Next time xyph warns me its risky, I'll know he means it. Now to my problem, how do I use this foreach loop without disabling the user of using simple html tags? Here's the file (editText.php) where the foreach loop was used: <?php session_start(); // THE LOOP WAS USED HERE BUT I REMOVED IT DUE TO THE USERS PROBLEM. function getSlug( $page ) { $page = strip_tags( $page ); preg_match_all( "/([a-z0-9A-Z-_]+)/", $page, $matches ); $matches = array_map( "ucfirst", $matches[0] ); $slug = implode( "-", $matches ); return $slug; } $fieldname = $_REQUEST['fieldname']; $encrypt_pass = @file_get_contents("files/password"); if ($_COOKIE['wondercms']!=$encrypt_pass) { echo "You must login before using this function!"; exit; } $content = rtrim(stripslashes($_REQUEST['content'])); // if to only allow specified tags if($fieldname=="title") $content = strip_tags($content); else $content = strip_tags($content,"<audio><source><embed><iframe><p><h1><h2><h3><h4><h5><h6><a><img><u><i><em><strong><b><strike><center><pre>"); $content = trim($content); $content = nl2br($content); if(!$content) $content = "Please be sure to enter some content before saving. Just type anything in here."; $content = preg_replace ("/%u(....)/e", "conv('\\1')", $content); if($fieldname>0 && $fieldname<4) $fname = "attachment$fieldname"; else $fname = $fieldname; $file = @fopen("files/$fname.txt", "w"); if(!$file) { echo "<h2 style='color:red'>*** ERROR *** unable to open content_$fieldname</h2><h3>But don't panic!</h3>". "Please set the correct read/write permissions to the files folder.<br/> Find the /files/ folder and CHMOD it to 751.<br /><br /> If this still gives you problems, open up the /files/ folder, select all files and CHMOD them to 640.<br /><br /> If this doesn't work, contact me <a href='http://krneky.com/en/contact'>right here</a>."; exit; } fwrite($file, $content); fclose($file); echo $content; // convert udf-8 hexadecimal to decimal function conv($hex) { $dec = hexdec($hex); return "&#$dec;"; } ?> Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/ Share on other sites More sharing options...
Zurev Posted May 8, 2011 Share Posted May 8, 2011 You can use strip_tags on the input first, to allow only certain html tags, and then htmlentities it. http://www.php.net/manual/en/function.strip-tags.php Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212502 Share on other sites More sharing options...
r0b Posted May 8, 2011 Author Share Posted May 8, 2011 If you check the code you can see I already strip the content for the added tags: if($fieldname=="title") $content = strip_tags($content); else $content = strip_tags($content,"<audio><source><embed><iframe><p><h1><h2><h3><h4><h5><h6><a><img><u><i><em><strong><b><strike><center><pre>"); How would I connect htmlentities and this? Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212509 Share on other sites More sharing options...
Zurev Posted May 8, 2011 Share Posted May 8, 2011 If you check the code you can see I already strip the content for the added tags: if($fieldname=="title") $content = strip_tags($content); else $content = strip_tags($content,"<audio><source><embed><iframe><p><h1><h2><h3><h4><h5><h6><a><img><u><i><em><strong><b><strike><center><pre>"); How would I connect htmlentities and this? You have it backwards... strip_tags' second parameter is ALLOWABLE tags, you want to allow embed and iframes? I'm assuming not... You wouldn't connect it, you would just call it afterwards. i.e. Run the strip_tags version through htmlentities. Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212510 Share on other sites More sharing options...
r0b Posted May 9, 2011 Author Share Posted May 9, 2011 I want to allow people to use those yeah, iframe (one reported using it) and embedding (youtube videos of course). Okay, so if I get it right, the code should look something like: strip_tags($content, htmlentities($key)); I hope I'm heading in the right direction as I've dealt with combining strip_tags and htmlentities together. I really appreciate the help Zurev, thanks for bearing with me. Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212594 Share on other sites More sharing options...
r0b Posted May 9, 2011 Author Share Posted May 9, 2011 I've been trying this but no success (yet), this code htmlentities(strip_tags($content),ENT_QUOTES); isn't doing the job. Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212627 Share on other sites More sharing options...
Zurev Posted May 9, 2011 Share Posted May 9, 2011 I've been trying this but no success (yet), this code htmlentities(strip_tags($content),ENT_QUOTES); isn't doing the job. You realize embedding and iframing could be used so maliciously it's not even funny? I mean, an iframe is literally a window TO ANOTHER WEBPAGE. lmao. Sorry just had to point that out. First off, I recommend modifying that loop, using all of REQUEST is not necessary, I mean that means you're even running $_GET through this, when really the data you want to sanitize is most likely $_POST data. For $_GET, a great deal can be handled through whitelisting and typecasting, I'm sure a lot of your stuff is whatever.php?action=post&id=1 - So the actions or whatever you're $_GET key is can be whitelisted into an array and checked to be in there, and you can typecast the $_GET[id] to integer forcing it to be an integer. As for $_POST, just making sure you're following the FIEO rule, filter input, escape output. Pretty much standards are, only allow the input you can allow, this means using prepared statements/mysql_real_escape_string. Escaping output is all about displaying the information to the browser, to that extent you would generally want to translate ALL html into it's entities, however you want to allow certain tags. I would allow the most basic of tags, p, b, i, etc. And have your own code for embedding things, notice in open source projects, when you want to embed a video, you don't just throw an embed tag in there, you do something like [embed=url] and their software processes it and I'm sure validates it in someway to make sure it isn't malicious and then converts that to the html code to embed on output. As for looping through $_POST and doing it that way, I'm sure if you want some sort of flexibility this will fail, there are going to be times when you do want html to go through, and you may end up having to html_entity_decode, but that's for a later discussion . foreach ($_POST as $postKey => $postVal) { $_POST[$postKey] = strip_tags($postVal, "<p><b><i>"); $_POST[$postKey] = htmlentities($postVal, ENT_QUOTES, "UTF-8"); } Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1212647 Share on other sites More sharing options...
r0b Posted May 15, 2011 Author Share Posted May 15, 2011 I've been trying this but no success (yet), this code htmlentities(strip_tags($content),ENT_QUOTES); isn't doing the job. You realize embedding and iframing could be used so maliciously it's not even funny? I mean, an iframe is literally a window TO ANOTHER WEBPAGE. lmao. Sorry just had to point that out. First off, I recommend modifying that loop, using all of REQUEST is not necessary, I mean that means you're even running $_GET through this, when really the data you want to sanitize is most likely $_POST data. For $_GET, a great deal can be handled through whitelisting and typecasting, I'm sure a lot of your stuff is whatever.php?action=post&id=1 - So the actions or whatever you're $_GET key is can be whitelisted into an array and checked to be in there, and you can typecast the $_GET[id] to integer forcing it to be an integer. As for $_POST, just making sure you're following the FIEO rule, filter input, escape output. Pretty much standards are, only allow the input you can allow, this means using prepared statements/mysql_real_escape_string. Escaping output is all about displaying the information to the browser, to that extent you would generally want to translate ALL html into it's entities, however you want to allow certain tags. I would allow the most basic of tags, p, b, i, etc. And have your own code for embedding things, notice in open source projects, when you want to embed a video, you don't just throw an embed tag in there, you do something like [embed=url] and their software processes it and I'm sure validates it in someway to make sure it isn't malicious and then converts that to the html code to embed on output. As for looping through $_POST and doing it that way, I'm sure if you want some sort of flexibility this will fail, there are going to be times when you do want html to go through, and you may end up having to html_entity_decode, but that's for a later discussion . foreach ($_POST as $postKey => $postVal) { $_POST[$postKey] = strip_tags($postVal, "<p><b><i>"); $_POST[$postKey] = htmlentities($postVal, ENT_QUOTES, "UTF-8"); } Thank you very much for replying with such an explanation. I might have found a more simple way of solving it, which wont increase the size of the CMS where every kB counts. Marking this as solved. Thanks again Zurev, you saved me a couple of days worth programming. Link to comment https://forums.phpfreaks.com/topic/235869-xss-vulnerability-part-2/#findComment-1215665 Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.