Page MenuHomePhabricator

XSS in I18nTags extension tag hooks
Closed, ResolvedPublic

Description

Using a PHP_CodeSniffer sniff I wrote (see T171520), I was looking for bugs arising from the left associativity of PHP's ternary operator, and I happened across I18nTags::linkTrail(). While I was considering how to fix the bug, it occurred to me that this function is a parser tag hook that includes unescaped user input as part of its HTML output. The same is true for the three other tag hooks (formatnum, grammar, and plural).

This XSS vulnerability can be reproduced on translatewiki.net. You do not need an account. Just go to Special:ExpandTemplates, enter <formatnum><script>alert(1)</script></formatnum>, <grammar><script>alert(1)</script></grammar>, <plural><script>alert(1)</script></plural>, or <linktrail><script>alert(1)</script></linktrail> in the "Input wikitext:" box, and click "OK".

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptAug 2 2018, 6:53 AM
Legoktm added a subscriber: Legoktm.Aug 2 2018, 7:16 AM

Just for later reference, phan-taint-check-plugin catches this as well:

<checkstyle version="6.5">
  <file name="./I18nTags_body.php">
    <error line="15" severity="warning" message="Outputting user controlled HTML from Parser tag hook \I18nTags::formatNumber (Caused by: ./I18nTags_body.php +12)" source="SecurityCheck-XSS"/>
    <error line="22" severity="warning" message="Outputting user controlled HTML from Parser tag hook \I18nTags::grammar (Caused by: ./I18nTags_body.php +18; ./I18nTags_body.php +19; ./I18nTags_body.php +18)" source="SecurityCheck-XSS"/>
    <error line="44" severity="warning" message="Outputting user controlled HTML from Parser tag hook \I18nTags::plural (Caused by: ./I18nTags_body.php +37)" source="SecurityCheck-XSS"/>
    <error line="63" severity="warning" message="Outputting user controlled HTML from Parser tag hook \I18nTags::linktrail (Caused by: ./I18nTags_body.php +47)" source="SecurityCheck-XSS"/>
  </file>
</checkstyle>

I've patched this in translatewiki.net by adding $parser->recursiveTagParse( $text, $frame );. To all of them.

I am surprised by how many sites use this extensions: https://wikiapiary.com/wiki/Extension:Parser_i18n_tags

What procedure should we follow here? Should I submit a patch to Gerrit, or should we wait for a general security release? This extension is not bundled with MediaWiki.

Bawolff added a subscriber: Bawolff.Aug 2 2018, 5:14 PM

What procedure should we follow here? Should I submit a patch to Gerrit, or should we wait for a general security release? This extension is not bundled with MediaWiki.

Since its not bundled with MediaWiki, we don't include it with the security release, so you shouldn't wait for that.

Generally the following procedure is followed:

  • Write a patch fixing the issue. Also make the patch up the version number. Commit message should start with the word SECURITY:
  • Get someone to review the patch before submitting it into gerrit (so that it can be immediately +2'd)
  • submit patch. Immediately +2.
  • Backport patch to all REL_XX branches that correspond to mediawiki branches still in support
  • Make this bug public
  • Send an announcement to mediawiki-l (For extensions that are not bundled with core we do not send an announcement to mediawiki-announce, but the people on mediawiki-l do appreciate an annoncement)

Thanks to @Petar.petkovic for review and support.

Nikerabbit changed the visibility from "Custom Policy" to "Public (No Login Required)".Aug 7 2018, 12:18 PM
Petar.petkovic triaged this task as High priority.Aug 7 2018, 2:35 PM
Arrbee closed this task as Resolved.Aug 20 2018, 7:16 AM
Arrbee assigned this task to Nikerabbit.