Reedy has submitted this change and it was merged. ( https://gerrit.wikimedia.org/r/391378 )
Change subject: SECURITY: Ensure Message::rawParams can't lead to XSS ...................................................................... SECURITY: Ensure Message::rawParams can't lead to XSS If you used wfMessage( 'foo' )->rawParams( 'bar"baz' ) there's a possibility of leading to xss, if the foo message has a $1 in an attribute, as the quote characters may end the attribute. To prevent that, we convert $1 to $'"1 for after parameters, so if any of them end up in attributes, the attribute escaping will break the parameter name, preventing substitution. This would of course break if someone intentionally inserted a raw parameter into an attribute, but that's silly and I don't think we should allow that. This is similar to the parser strip marker issue. Bug: T176247 Change-Id: If83aec01b20e414f9c92be894f145d7df2974866 --- M RELEASE-NOTES-1.27 M includes/Message.php 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/RELEASE-NOTES-1.27 b/RELEASE-NOTES-1.27 index 1936d73..2f7a2e9 100644 --- a/RELEASE-NOTES-1.27 +++ b/RELEASE-NOTES-1.27 @@ -23,6 +23,7 @@ * (T165846) SECURITY: BotPassword login attempts weren't throttled. * (T128209) SECURITY: Reflected File Download from api.php. * (T134100) SECURITY: Do not reveal if user exists during login failure. +* (T176247) SECURITY: Ensure Message::rawParams can't lead to XSS. == MediaWiki 1.27.3 == Due to a packaging error, the wrong version of the SyntaxHighlight extension was diff --git a/includes/Message.php b/includes/Message.php index 303a4b9..dc311fb 100644 --- a/includes/Message.php +++ b/includes/Message.php @@ -1064,11 +1064,29 @@ * @return string */ protected function replaceParameters( $message, $type = 'before' ) { + // A temporary marker for $1 parameters that is only valid + // in non-attribute contexts. However if the entire message is escaped + // then we don't want to use it because it will be mangled in all contexts + // and its unnessary as ->escaped() messages aren't html. + $marker = $this->format === 'escaped' ? '$' : '$\'"'; $replacementKeys = []; foreach ( $this->parameters as $n => $param ) { list( $paramType, $value ) = $this->extractParam( $param ); - if ( $type === $paramType ) { - $replacementKeys['$' . ( $n + 1 )] = $value; + if ( $type === 'before' ) { + if ( $paramType === 'before' ) { + $replacementKeys['$' . ( $n + 1 )] = $value; + } else /* $paramType === 'after' */ { + // To protect against XSS from replacing parameters + // inside html attributes, we convert $1 to $'"1. + // In the event that one of the parameters ends up + // in an attribute, either the ' or the " will be + // escaped, breaking the replacement and avoiding XSS. + $replacementKeys['$' . ( $n + 1 )] = $marker . ( $n + 1 ); + } + } else { + if ( $paramType === 'after' ) { + $replacementKeys[$marker . ( $n + 1 )] = $value; + } } } $message = strtr( $message, $replacementKeys ); -- To view, visit https://gerrit.wikimedia.org/r/391378 To unsubscribe, visit https://gerrit.wikimedia.org/r/settings Gerrit-MessageType: merged Gerrit-Change-Id: If83aec01b20e414f9c92be894f145d7df2974866 Gerrit-PatchSet: 1 Gerrit-Project: mediawiki/core Gerrit-Branch: REL1_27 Gerrit-Owner: Reedy <re...@wikimedia.org> Gerrit-Reviewer: Brian Wolff <bawolff...@gmail.com> Gerrit-Reviewer: Reedy <re...@wikimedia.org> _______________________________________________ MediaWiki-commits mailing list MediaWiki-commits@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits