r110045 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r110044‎ | r110045 | r110046 >
Date:02:15, 26 January 2012
Author:neilk
Status:reverted (Comments)
Tags:niklas 
Comment:
sanitize outgoing messages
Modified paths:
  • /trunk/phase3/includes/MessageBlobStore.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/MessageBlobStore.php
——@@ -350,7 +350,12 @@
351351 $messages = array();
352352
353353 foreach ( $module->getMessages() as $key ) {
354 - $messages[$key] = wfMsgExt( $key, array( 'language' => $lang ) );
 354+ $messages[$key] =
 355+ Sanitizer::normalizeCharReferences(
 356+ Sanitizer::removeHTMLtags(
 357+ wfMsgExt( $key, array( 'language' => $lang ) )
 358+ )
 359+ );
355360 }
356361
357362 return FormatJson::encode( (object)$messages );

Follow-up revisions

RevisionCommit summaryAuthorDate
r112526Revert r110045: well-meaning but broken attempt to apply preemptive XSS prote...brion21:52, 27 February 2012

Comments

#Comment by NeilK (talk | contribs)   02:19, 26 January 2012

Trying to be extra careful with what we emit. While translations have never been a source of XSS before, we should close the hole before it's exploited.

I tested this change on the most complicated ResourceLoader page I know of (UploadWizard) and the only differences were all legitimate XSS problems.

n.b. Sanitizer::removeHTMLtags does not actually remove tags. It really just ensures that some good tags like are balanced, and that evil tags like <script> are escaped with HTML entities, and catches various other XSS issues.

#Comment by NeilK (talk | contribs)   02:19, 26 January 2012

Right, that should be good tags like '<b>'. A perfect example of that function in action.

#Comment by NeilK (talk | contribs)   02:20, 26 January 2012

Closing tags

#Comment by Brion VIBBER (talk | contribs)   21:47, 27 February 2012

Causes bug 34708 -- usage examples in WikiEditor help are broken!

Status & tagging log