Extension talk:AddScript/Archive 1

XSS Vulnerability Description
Version 1.1 is vulnerable to XSS attacks. Consider the following: Hopefully the above will help the author correct this very serious problem. --Jimbojw 19:37, 29 March 2007 (UTC)
 * 1) Malicious user creates a page called "EvilScript" and fills it with evil JavaScript (perhaps adding another &lt;script&gt; tag to the DOM, etc).
 * 2) Malicious user creates a page containing the following: &lt;addscript src='../index.php?title=EvilScript&action=raw&ctype=text/javascript'/&gt;
 * Note: This could even be the same page (for example in a JS comment), so an admin going there for the first time to check it out/delete it could be hit immediately!
 * 1) This becomes the following in the document HEAD: &lt;script src="/wikiroot/jsscripts/../index.php?title=EvilScript...
 * Note: Modern browsers will intelligently strip '/../' along with the parent dir when found in fetched URLs.
 * 1) Innocent visitor visits the containing page, which loads EvilScript - user's account is compromised.


 * So, is the fix for this sort of vulnerability to remove the '/../' in the user provided string? Is this 'good enough' since any other URL must be part of the sysop controlled 'jsscripts' path? Jean-Lou Dupont 12:37, 30 March 2007 (UTC)


 * Disallowing the patterns '/./' and '/../' in the URL might be good enough - I'm not sure. If you intend for all the scripts to be directly under the 'jsscripts' directory (ie no subdirectories), then it might be sufficient to strip out all forward slashes.


 * As a side note, if you continue allowing users to specify information that's to become part of a URL, you should really urlencode the input. As it stands now, even if you solved the '/../' problem, people could still specify a double-quote to escape the src attribute, then specify other attributes within the &lt;script&gt; tag.  I'm not sure how effective this would be as an attack vector, but it could be done.


 * Regarding a more robust solution to the current dilemma: since you're requiring file-system level access in the first place (to put the scripts in the jsscripts dir), you could have a mapping of short-hand strings to full paths. For example, instead of this: &lt;addscript src="path/to/whatever.js" /&gt; You could have the user provide this: &lt;addscript name="whatever" /&gt;  Then, in the configuration for the extension (LocalSettings.php), you'd have a mapping of names to paths.  Continuing with this example, you'd use this: $wgAddScriptMappings = array( 'whatever' => 'path/to/whatever.js' );


 * Implementation of any or all of the above recommendations is left as an exercise for the reader :) --Jimbojw 20:44, 30 March 2007 (UTC)

What Jimbojw suggests is quite simmilar to the precautions i took with Extension:HTMLets. It's actually quite simmilar to this, only that it reads HTML from files and inserts it into pages directly, instead of referencing scripts via URL. But the code would look simmilar, I guess. -- Duesentrieb ⇌ 23:52, 30 March 2007 (UTC)


 * I have uploaded v1.2, please check it out. I have tested it on my home windows system as well as on my 1and1 Linux hosted site. Jean-Lou Dupont 01:10, 31 March 2007 (UTC)

Script doesn't work on bluehost.com...
...but with a small mod it can do. Instead of return file_exists( $bpath."/{$spath}" ); if you use return file_exists( self::$base.$uri.".js" ); then things seem to work. I haven't had time to figure out which PHP settings cause this, maybe will come back to this later. - all the best, Adam

Which version of PHP are you running on bluehost.com ? There isn't much code behing the code line you are pointing in your snippet... only 'dirname' based statements... Jean-Lou Dupont 19:05, 21 April 2007 (UTC)