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)
 * The code snippets were regarding the CheckURI function in your code. My PHP version is 5.1.6 (although bluehost's default PHP is 4.something - I just asked to be upgraded so that MediaWiki would install).  Do you want me to send you phpinfo? --Amacieli 09:34, 24 April 2007 (UTC)
 * Another question: Just noticed that headscripts are only loaded if I use &action=purge in the URL. I don't think this is a cache setting problem as my other wiki pages update just fine.  What I'm doing that's not working is using as the first line in my wiki page.  Any ideas?  --Adam 12:19, 24 April 2007 (UTC)
 * This is probably 'parser cache' related. Please use Parser Cache Control extension. Alternatively, give a try to version 1.3 of the extension. Jean-Lou Dupont 15:14, 24 April 2007 (UTC)
 * What Adam is describing (with regards to the action=purge statment) is still the case with 1.3. The problem is that any modifications to $wgOut</tt> that a parser extension makes (even a hook to ParserBeforeTidy) will expire as soon as the output is cached.
 * The good news is that there are ways around this without forcing a purge on ever page view. I discuss one such method in my article Doing more with MediaWiki parser extensions. Hope this helps :) --Jimbojw 18:38, 24 April 2007 (UTC)
 * I went too fast in modifying the code for releasing v1.3 (release 1.3 still works OK but wasn't really necessary): I had analysed the problem and came to the conclusion that the answer was Extension:ParserCacheControl *if* one wants to retain all the power of the 'parser cache' functionality without going through additional hoops (thanks anyways Jimbojw ) Jean-Lou Dupont 18:53, 24 April 2007 (UTC)
 * No problem. Another option would be to just expire the cache yourself. I believe this is possible to do from the parser tag code. In fact there used to be an extension - called PurgePage? - that added a tag called &lt;purge&gt;</tt> which would do just that.  I'm not sure if PurgePage is still around or not.
 * For what it's worth, I strongly encourage you to find a way to not force the user to constantly purge/bypass the cache. For large, popular pages on wikis with appreciable amounts of traffic, it may not be feasible to disable caching due to performance losses. --Jimbojw 22:09, 24 April 2007 (UTC)
 * I gave Jimbojw's code a whirl, and it works as advertised, you can just refresh your browser without purging, and the head script stays. I just replaced the addMeta line with addScript so that I got what I needed.  Thanks for all your help, Jimbojw and JLD!  --Adam 02:29, 25 April 2007 (UTC)