SIRNOT! Html Divs causing problems in posts

garymardell

ok, i was just wondering what is stopping someone writing a div that will blank out the whole screen with white on every posts?

http://mad4gaming.co.uk/comments.php?DiscussionID=10&page=1#Item_1

garymardell

i made a better example, with the combination of accout pictures a user could upload a really disgusting image and use a div to paste it over the comment,

view here http://mad4gaming.co.uk/comments.php?DiscussionID=11&page=1#Item_1 dont worry its not disgusting as it is a clan forum and all and i wouldnt want to look at it either

lech

From the look of it, apparently nothing is stopping it. I presume you've allowed for raw html to get through.

garymardell

i used the html formatter and thats it, you can do it on this forum. I could make this thread just a page of repeating bunnys with pancakes on their heads

lech

Yeah, I think to some extent sirnot didn't anticipate css hacks and the like of slipping through the way that they are. This is where the raw-html formatter allowed users to perform such things. I'd say tweak the topic to include "SirNot: I have html causing problems in posts" and he'll probably tweak things as soon as he finds this and the other post detailing the extent of potential damage which can arise.

Inky

That's the only thing that stopped me from using the html extension on my forum. Pity I never thought to mention it here. :P

Would stripping out the style property on tags be easy to do?

lech

easy for those who fluently speak regexp. Right now, it's simply stripping javascript, and allowing most html to pass.

bjrn

Perhaps it would be even easier to disallow all tag attributes? I can't think of a time where you would really need it when posting a comment somewhere. That way you could strip everything after a space in a tag.

Edit: I just thought of links, which use attributes, and would be quite useless without them.

NickE

I'm sorry the html formatter dosn't stop things like that, but otherwise it'd have to completely parse both the html and css (something which would take up way too much time, not to mention space and effort), and even then determing what could foul up the layout wouldn't be 100% accurate. But yes, you could get rid of the style attribute, if you wanted. Just remove the do {} while(); loop in HtmlFormatter::RemoveEvilAttribs then add in a regexp to remove style attributes.

garymardell

any chance you could expand on that are post the code?

Dinoboff

That is not really a problem. There is no security issue with this code. This is just annoying. You just need to turn on the test-only mode, change the code and banned the user.

NickE

Sorry, I was sort of flustered there. Although I haven't tested it yet, I believe that replacing the RemoveEvilAttribs function with this will do what you were thinking of:function RemoveEvilAttribs($String) { $P = array( "/(\s+?)(href|src|background|url|dynsrc|lowsrc)\s*=(\W*)(.+?):([^\\3]+?)/ei", "/(\s+?)on([\w]+)\s*=(.+?)/i", "/style\s*=(\W*)(.+)\\1/si" ); $R = array( '$this->RemoveQuoteSlashes(\'\\1\\2=\\3\').(in_array(strtolower(\'\\4\'), $this->AllowedProtocols) ? \'\\4:\' : $this->DefaultProtocol).$this->RemoveQuoteSlashes(\'\\5\')', '\\1On\\2=\\3', '' ); $sReturn = preg_replace($P, $R, $String); return $sReturn; }

elbiggus

I know I'm late to the party, but as the fix doesn't appear to be turned on in the add-on by default I thought I'd chime in anyway -- perhaps more worrying than the potential for people to plaster images everywhere, etc., is that with a bit of work it's possible to "spoof" posts from other users in a way that's only noticeable if you happen to be browsing in text-only mode. I spent a while playing with it earlier -- someone tried to insert a video from YouTube and got an extra closing DIV in there somehow, and as soon as I realised DIVs went through unfiltered I immediately hit upon the spoofing idea. How this wasn't anticipated as a problem is beyond me, really.

(I also notice that the fix hasn't been applied here, so if the implications aren't obvious then I'll happily demonstrate!)

NickE

Open up the extension file and you should see three definitions, which are boolean options you can change (you should update to the latest version if you haven't already done that). For instance, if you don't want people to be able to style tags, you can set HTML_ALLOW_INLINE_STYLING to 0 instead of 1.