Correctly escape uselang attribute to prevent xss
Closed, ResolvedPublic

Description

It looks like the "uselang" parameter is vulnerable to simple reflective xss attacks.

Fortunately, all modern browsers refuse to run javascript they see in the url, but there is a constant stream of ways around that protection.

http://en.wikipedia.org/wiki/Main_Page?uselang=a%27%20onmouseover=eval(alert(1))%20e=%27

It looks like we use the uselang parameter in several single-quoted strings, but don't escape single quotes in it.


Version: unspecified
Severity: major

bzimport set Reference to bz36938.
csteipp created this task.Via LegacyMay 17 2012, 4:57 PM
Catrope added a comment.Via ConduitMay 18 2012, 7:07 PM

Draft submitted in Gerrit: https://gerrit.wikimedia.org/r/#/c/7979/

This draft is private so you won't be able to view it by default, even if you are a Gerrit administrator. I shared it with Chris, Tim and Sam; if someone else would like to review it too, ask me to share it with you.

Reedy added a comment.Via ConduitJun 1 2012, 4:08 PM
  • Bug 37275 has been marked as a duplicate of this bug. ***
GreenReaper added a comment.Via ConduitJun 14 2012, 4:02 AM

The fix for this appears to cause a couple of PHP Notices (at least on 1.17.5):

Notice: Undefined variable: userlang in [..]/includes/SkinTemplate.php on line 327
Notice: Undefined variable: userdir in [..]/includes/SkinTemplate.php on line 328

This is on a Russian-language wiki with my user language set to English.

The lines concerned are intended to set the lang and dir attributes . . .

$lang = $wgLang->getCode();
$dir = $wgLang->getDir();
if ( $lang !== $wgContLang->getCode() || $dir !== $wgContLang->getDir() ) {

$escUserlang = htmlspecialchars( $userlang );   << HERE
$escUserdir = htmlspecialchars( $userdir );     << HERE
// Attributes must be in double quotes because htmlspecialchars() doesn't
// escape single quotes
$attrs = " lang=\"$escUserlang\" dir=\"$escUserdir\"";
$tpl->set( 'userlangattributes', $attrs );

It looks like $lang and $dir were renamed to $userlang and $userdir in 1.18; the patch for 1.17 should be corrected to refer to $lang and $dir.

Reedy added a comment.Via ConduitJun 14 2012, 1:54 PM

(In reply to comment #3)

The fix for this appears to cause a couple of PHP Notices (at least on 1.17.5):

Notice: Undefined variable: userlang in [..]/includes/SkinTemplate.php on line
327
Notice: Undefined variable: userdir in [..]/includes/SkinTemplate.php on line
328

This is on a Russian-language wiki with my user language set to English.

The lines concerned are intended to set the lang and dir attributes . . .

$lang = $wgLang->getCode();
$dir = $wgLang->getDir();
if ( $lang !== $wgContLang->getCode() || $dir !== $wgContLang->getDir() ) {

$escUserlang = htmlspecialchars( $userlang );   << HERE
$escUserdir = htmlspecialchars( $userdir );     << HERE
// Attributes must be in double quotes because htmlspecialchars() doesn't
// escape single quotes
$attrs = " lang=\"$escUserlang\" dir=\"$escUserdir\"";
$tpl->set( 'userlangattributes', $attrs );

It looks like $lang and $dir were renamed to $userlang and $userdir in 1.18;
the patch for 1.17 should be corrected to refer to $lang and $dir.

https://gerrit.wikimedia.org/r/11284

Also note, 1.17 is due to go EOL this month.

duplicatebug added a comment.Via ConduitJun 17 2012, 6:48 AM

(In reply to comment #2)

  • Bug 37275 has been marked as a duplicate of this bug. ***

Please make dup also visible. Thanks.

Krinkle added a comment.Via ConduitJun 17 2012, 8:32 PM

(In reply to comment #5)

(In reply to comment #2)
> * Bug 37275 has been marked as a duplicate of this bug. *

Please make dup also visible. Thanks.

Done. Moved to Product:MediaWiki

csteipp added a project: Security.Via WebThu, Mar 26, 8:39 PM

Add Comment

Column Prototype
This is a very early prototype of a persistent column. It is not expected to work yet, and leaving it open will activate other new features which will break things. Press "\" (backslash) on your keyboard to close it now.