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:

• 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 ParserFirstCallInit .
• That foreach to loop over args is completely unnecessary. You can simply use $color = isset( $argv['color'] ) ? $argv['color'] : '50A6C2';. (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 $wgExtensionAssetsPath 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 Html::element( 'param', array( 'name' => "bgcolor", 'value' => "#{$bg}" ) ); to build those <param/>'s. You can use Html::rawElement for the few things like the <object> where you do in fact want to nest more html building methods inside of it.
  • Your error div should be built using return Html::element( 'div', array( 'class' => "noprint" ), "missing MP3 file, file not found: $input." );.
  • 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: Html::element( 'param', array( 'name' => "FlashVars", 'value' => wfArrayToCGI( array( 'mp3' => $mp3, 'bgcolor' => $bg, 'loadingcolor' => $color, 'buttoncolor' => $color, 'slidercolor' => $color ) ) ) ) (of course with more clean indentation; make sure you drop the urlencode from the $mp3 = urlencode($input); when you do this since wfArrayToCGI will do the urlencoding for you).

Daniel Friesen (Dantman) 01:00, 3 January 2012 (UTC)

Since you updated the extension I thought I might follow up:

• 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.

Daniel Friesen (Dantman) 08:05, 4 January 2012 (UTC)

•  Done move file to extensions/MiniMp3/MiniMp3.php instead.
•  Done You should avoid using $wgScriptPath inside of the description message.
•  Done Don't use wgExtensionFunctions, use ParserFirstCallInit .
•  Done That foreach to loop over args is completely unnecessary.
•  Done Drop the ?> from the end of the file.
•  Done Don't use $wgUploadPath for the download link.
•  Done XSS - To properly escape everything you should make some changes:
  •  Done use our Html:: class for params
  • use Html::rawElement for the few things like the <object>
  •  Done use of our wfArrayToCGI method

updated

•  Done Don't use $wgParser when using ParserFirstCallInit
• The XSS and most of the other issues still stand.

redekopmark 04:41, 5 January 2012 (UTC)

Good improvements. "{$flashFile}" isn't particularly necessary, $flashFile is enough given there's nothing else in the string. It's good practice to never leave something unescaped when you don't have a reason to. In this case it's best not to leave $flashFile unescaped inside your <object> tag. I suggest using Html::rawElement there (nest the rest of your pieced together html inside of the third argument and you can omit the </object> for the closing parenthesis). Since you added a media handler you may also want to consider combining your code into some common code instead of duplicating it. $flashFile should also be "$wgExtensionAssetsPath/MiniMp3/player_mp3_mini.swf". Daniel Friesen (Dantman) 05:00, 5 January 2012 (UTC)

Licensing Extension:MiniMp3

Gerrit change 343308 is attempting to add a license to the MP3MediaHandler which is derived from your work at Extension:MiniMp3. MarkAHershberger needs to get your permission to apply the GPLv3 license that he is suggesting. It would be great if you could work with Mark to come to an agreement on a suitable OSI approved license for the extension. --BDavis (WMF) (talk) 17:31, 13 May 2017 (UTC)

XSS Error W4GRatingbar

There are some issues with mediawiki 1.27.5 and the w4gratingbar 2.2 fork ... voting produces an error like: XSS attempt detected. Please make sure your browser isn't configured to hide referer information. How can we solve this? Kind regards Gerd

I've been pretty busy with other things and I don't have time for much mediawiki stuff anymore I think it was working fine for me with 1.27. any chance you're running on https? if so that's probably the problem and it's a simple fix, if you aren't I have no idea off the top of my head 96.63.159.141 13:26, 23 January 2019 (UTC)

Sounds good, we try https. maybe thas the simple solution. Its not configured at the moment beacuse we are not ready with other configuartions of the new server. But i will do the change now a little bit earlier and will inform you :) thanks. Gerd 15:14, 23 January 2019 (UTC)

SOLVED > it was an https and domain redirect problem of our server. extension works fine with MW 1.27.5 Gerd 14:11, 24 January 2019 (UTC)