HTML Formatter Exploit

NickE

Just as an example. The code used is
<img src="blabla.png" onerror="alert('Hello There');" width=0 height=0>
Looking through the HTML Formatter's code, I noticed it didn't replace a lot of the possible events one could hide javascript in. I'd recommend replacing all on* attributes in each tag with html entities, or just removing them all together.

Minisweeper

*strokes sirnots bug finding abilities and watches him take down the forum. p.s. this is the 1000th visible discussion :D

NickE

I'm not just brilliant with regular expressions, but replacing all of the on* string eregi_replace() functions with the following worked for me:
$String = preg_replace("#<(.*) on(.*)=(.*)>#si", "<\\1 &#111;n\\2=\\3>", $String);

lech

lol, nice find.

Mark

Wow - thanks, Sirnot :)

NickE

Sorry guys, just realized the first solution I posted dosn't work properly if there's one, no space in between on* and first attribute and/or two, there's more than on on* attribute. I think this one solves it, but like I said, I'm horrid with regular expressions. If anyone else is any good at this stuff, your help would be much appriciated.
$String = preg_replace(array("<(.*?)on(.*?)>", "<(.*?)On(.*?)>"), array("\\1&#111;n\\2", "\\1&#76;n\\2"), $String);
Whoops, that replaces all 'o's now. Hmm... this is getting annoying.

NickE

Ok, I think I actually got it this time. This should fix all js events as well as the multitudes of different ways one might get javascript to run. You want to replace your Execute() function within the HTML formatter class with the following (I didn't put in anything to replace newlines, but if you want it to just pop in an nl2br() function around the preg_replace()).

function Execute($String)
{
	$AllowedProtocols = array('http', 'ftp', 'https', 'irc', 'gopher');
	
	$Patterns = array(
		"/o(?i)(n)/", //block all js events, but keep it as exact as possible in case 
		"/O(?i)(n)/", //we're mistaking it for a url or something
		"/<a(.+?)href\s*=(\W*)([\w\d\x0a\x0d#&;]+?):([^>]+?)>/esi", 
			//on some browsers the js protocol will still work even if it
			//contains html entities or a newline seperating 'java' and 'script'
		"/s(?i)(cript)/", //now we can go through and cancel out any script tags
		"/S(?i)(cript)/"
	);
	$Replacements = array(
		"&#111;\\1", 
		"&#76;\\1", 
		'$this->CheckProtocol("\\3", $AllowedProtocols, "href="."\\2", "<a"."\\1", "\\4".">")', 
		"&#115;\\1", 
		"&#80;\\1"
	);
	
	return preg_replace($Patterns, $Replacements, $String);
}

Then you'd want to add in this function in the same class as well:

function CheckProtocol($Check, $Allow, $Extra, $Prefix, $Suffix)
{
	$sReturn = stripslashes($Prefix);
	if(!in_array($Check, $Allow)) $sReturn .= ($Extra.'http://');

	else $sReturn .= ($Extra.$Check.':');
	$sReturn .= stripslashes($Suffix);
	
	return $sReturn;
}

lol, just as I thought; the current HTML parser dosn't fix the fact that most browsers'll still parse the js protocol even if it has entities in it:
Click Me
So that's three bugs discovered so far, hopefully all fixed in this revision.

Mark

It's up here now. It did some strange stuff to your post there. I'll test a bit.

Mark

Okay - better. Thanks!

soupenvy

Is version 1.2 Of HTMLFormatter (the one included with Vanilla 0.9.3) the most recent version of the extension? Not sure wether to make these changes to HTML Formatter 1.2, looks like the info right here on the message boards is out of date.

ithcy

also, are you posting these fixes to the addons directory?

Mark

I'd bet dollars to donuts that this is out of date. Look at the date it was posted: Oct 26th, 2005

ithcy

oh jeez.

no wonder it didn't work

*gives mark donuts

nick1presta

hehe. i was inspired

soupenvy

Yeah, I assumed it was out of the date for that very reason. No date is in the extension itself though.

Mark

SirNot has been maintaining this extension for quite a while now. I've asked him to upload it to the add-on directory so updates can be seen.

LoOkOuT

Sir Not, is this extension ready for the add-on directory?