User talk:Redekopmark

Re: XSS extension check
You asked Johnduhart for an xss review of your extension, I noticed it and thought I'd do a bit of a full review of it: Daniel Friesen (Dantman) 01:00, 3 January 2012 (UTC)
 * Instead of having a mp3.php file you should have it installed inside of extensions/MiniMp3/MiniMp3.php instead. If your extension gets into svn this will also make things easier since you can just drop a copy of the .swf file in there as well.
 * You should avoid using $wgScriptPath inside of the description message. Especially given good extensions in svn put their messages inside of an .i18n.php file for translation where that won't exactly be available.
 * Don't use wgExtensionFunctions to register your tag, use.
 * That foreach to loop over args is completely unnecessary. You can simply use . (IMHO $params would be a better var name than $argv)
 * Drop the ?> from the end of the file, there's a good reason our code style has it omitted.
 * Don't use $wgUploadPath for the download link, you should provide a config variable that can be used to point to the proper url where that file would reside. Either that or make use of and have the file put inside of your extension folder.
 * As you asked, yes you do in fact have an XSS vulnerability inside of your extension. The bg and color parameters can be used for an xss attack. The way you use $input inside of that error message also looks potentially hazardous, though I can't quite understand what situation that $input would even have a proper value.
 * To properly escape everything you should make some changes:
 * You're best off making use of our Html:: class to build your markup instead of trying to concatenate everything. For example use  to build those 's. You can use Html::rawElement for the few things like the where you do in fact want to nest more html building methods inside of it.
 * Your error div should be built using.
 * To build those FlashVars I suggest making use of our wfArrayToCGI method it will take a clean array and turn it into a properly urlencoded string in the format you want and then passing it to Html::element will have the &'s properly html escaped, all nice and implicitly. Something like this:  (of course with more clean indentation; make sure you drop the urlencode from the   when you do this since wfArrayToCGI will do the urlencoding for you).

Since you updated the extension I thought I might follow up: Daniel Friesen (Dantman) 08:05, 4 January 2012 (UTC)
 * Don't use $wgParser when using ParserFirstCallInit, it defeats the purpose. Your callback is given a parser instance as an argument, so just add a $parser argument and call setHook on that.
 * The XSS and most of the other issues still stand.