Page MenuHomePhabricator

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

Details

Reference
bz36938

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:21 AM
bzimport set Reference to bz36938.

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.

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

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.

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

(In reply to comment #2)

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

Please make dup also visible. Thanks.

(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