Matmarex has uploaded a new change for review.
https://gerrit.wikimedia.org/r/64911
Change subject: displaytitle: reject some CSS if $wgRestrictDisplayTitle set
......................................................................
displaytitle: reject some CSS if $wgRestrictDisplayTitle set
$wgRestrictDisplayTitle is intended to make it possible to simply
copy-and-paste the title text even if it requires some styling like
subscript or superscript. Using a <span style="display: none;" />
broke that expectation, as the text hidden in such way becomes
completely invisible and unselectable. This patch rejects such styles.
Also disallowed 'user-select' and 'visibility', since they both
prevents the user from selecting and/or copying the text as well.
Minor change to Sanitizer::checkCss was made for it to pass through
values which consist of nothing but a single comment, to allow this
rejection to display some sort of a notification to the user.
Bug: 26547
Change-Id: Ie162535b6bcbebce4ee69f6dcc1957ccccc3c672
---
M includes/Sanitizer.php
M includes/parser/CoreParserFunctions.php
M tests/parser/parserTests.txt
3 files changed, 66 insertions(+), 20 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/11/64911/1
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index ed01235..71feef0 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -856,19 +856,24 @@
$value = preg_replace_callback( $decodeRegex,
array( __CLASS__, 'cssDecodeCallback' ), $value );
- // Remove any comments; IE gets token splitting wrong
- // This must be done AFTER decoding character references and
- // escape sequences, because those steps can introduce comments
- // This step cannot introduce character references or escape
- // sequences, because it replaces comments with spaces rather
- // than removing them completely.
- $value = StringUtils::delimiterReplace( '/*', '*/', ' ', $value
);
+ // Let the value through if it's nothing but a single comment,
to
+ // allow other functions which may reject it to pass some error
+ // message through.
+ if ( !preg_match( '!\^ \s* /\* [^*/]* \*/ \s* $ !x', $value ) ) {
+ // Remove any comments; IE gets token splitting wrong
+ // This must be done AFTER decoding character
references and
+ // escape sequences, because those steps can introduce
comments
+ // This step cannot introduce character references or
escape
+ // sequences, because it replaces comments with spaces
rather
+ // than removing them completely.
+ $value = StringUtils::delimiterReplace( '/*', '*/', '
', $value );
- // Remove anything after a comment-start token, to guard against
- // incorrect client implementations.
- $commentPos = strpos( $value, '/*' );
- if ( $commentPos !== false ) {
- $value = substr( $value, 0, $commentPos );
+ // Remove anything after a comment-start token, to
guard against
+ // incorrect client implementations.
+ $commentPos = strpos( $value, '/*' );
+ if ( $commentPos !== false ) {
+ $value = substr( $value, 0, $commentPos );
+ }
}
// Reject problematic keywords and control characters
diff --git a/includes/parser/CoreParserFunctions.php
b/includes/parser/CoreParserFunctions.php
index be945f7..fbd30ce 100644
--- a/includes/parser/CoreParserFunctions.php
+++ b/includes/parser/CoreParserFunctions.php
@@ -363,22 +363,35 @@
static function displaytitle( $parser, $text = '' ) {
global $wgRestrictDisplayTitle;
- #parse a limited subset of wiki markup (just the single quote
items)
+ // parse a limited subset of wiki markup (just the single quote
items)
$text = $parser->doQuotes( $text );
- #remove stripped text (e.g. the UNIQ-QINU stuff) that was
generated by tag extensions/whatever
+ // remove stripped text (e.g. the UNIQ-QINU stuff) that was
generated by tag extensions/whatever
$text = preg_replace( '/' . preg_quote( $parser->uniqPrefix(),
'/' ) . '.*?'
. preg_quote( Parser::MARKER_SUFFIX, '/' ) . '/', '',
$text );
- #list of disallowed tags for DISPLAYTITLE
- #these will be escaped even though they are allowed in normal
wiki text
+ // list of disallowed tags for DISPLAYTITLE
+ // these will be escaped even though they are allowed in normal
wiki text
$bad = array( 'h1', 'h2', 'h3', 'h4', 'h5', 'h6', 'div',
'blockquote', 'ol', 'ul', 'li', 'hr',
'table', 'tr', 'th', 'td', 'dl', 'dd', 'caption', 'p',
'ruby', 'rb', 'rt', 'rp', 'br' );
- #only requested titles that normalize to the actual title are
allowed through
- #if $wgRestrictDisplayTitle is true (it is by default)
- #mimic the escaping process that occurs in
OutputPage::setPageTitle
- $text = Sanitizer::normalizeCharReferences(
Sanitizer::removeHTMLtags( $text, null, array(), array(), $bad ) );
+ // disallow some styles that could be used to bypass
$wgRestrictDisplayTitle
+ $htmlTagsCallback = !$wgRestrictDisplayTitle ? null : function
( $params ) {
+ if ( isset( $params['style'] ) ) {
+ // this is called later anyway, but we need it
right now for the regexes below to be safe
+ // calling it twice doesn't hurt
+ $params['style'] = Sanitizer::checkCss(
$params['style'] );
+
+ if ( preg_match(
'/(display|user-select|visibility)\s*/i', $params['style'] ) ) {
+ $params['style'] = '/* attempt to
bypass $wgRestrictDisplayTitle */';
+ }
+ }
+ }
+
+ // only requested titles that normalize to the actual title are
allowed through
+ // if $wgRestrictDisplayTitle is true (it is by default)
+ // mimic the escaping process that occurs in
OutputPage::setPageTitle
+ $text = Sanitizer::normalizeCharReferences(
Sanitizer::removeHTMLtags( $text, $htmlTagsCallback, array(), array(), $bad ) );
$title = Title::newFromText( Sanitizer::stripAllTags( $text ) );
if ( !$wgRestrictDisplayTitle ) {
diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
index d5e221a..011e5e9a 100644
--- a/tests/parser/parserTests.txt
+++ b/tests/parser/parserTests.txt
@@ -13290,6 +13290,34 @@
!! end
!! test
+Verify that displaytitle handles inline CSS styles (bug 26547) - rejected value
+!! options
+showtitle
+title=[[Screen]]
+!! config
+wgAllowDisplayTitle=true
+wgRestrictDisplayTitle=true
+!! input
+{{DISPLAYTITLE:<span style="display: none;">s</span>creen}}
+!! result
+<span style="/* attempt to bypass $wgRestrictDisplayTitle */">s</span>creen
+!! end
+
+!! test
+Verify that displaytitle handles inline CSS styles (bug 26547) - accepted value
+!! options
+showtitle
+title=[[Screen]]
+!! config
+wgAllowDisplayTitle=true
+wgRestrictDisplayTitle=true
+!! input
+{{DISPLAYTITLE:<span style="color: red;">s</span>creen}}
+!! result
+<span style="color:red;">s</span>creen
+!! end
+
+!! test
preload: check <noinclude> and <includeonly>
!! options
preload
--
To view, visit https://gerrit.wikimedia.org/r/64911
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie162535b6bcbebce4ee69f6dcc1957ccccc3c672
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Matmarex <matma....@gmail.com>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
