[MediaWiki-commits] [Gerrit] displaytitle: reject some CSS if $wgRestrictDisplayTitle set - change (mediawiki/core)
jenkins-bot has submitted this change and it was merged.
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
prevent the user from selecting and/or copying the text as well.
Minor changes in Sanitizer:
* checkCss() was made 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.
* encodeTagAttributes() was added as a counterpart to
decodeTagAttributes(), pulling some code out of fixTagAttributes().
Bug: 26547
Change-Id: Ie162535b6bcbebce4ee69f6dcc1957c3c672
---
M includes/DefaultSettings.php
M includes/Sanitizer.php
M includes/parser/CoreParserFunctions.php
M tests/parser/parserTests.txt
M tests/phpunit/includes/SanitizerTest.php
5 files changed, 113 insertions(+), 36 deletions(-)
Approvals:
Mattflaschen: Looks good to me, approved
jenkins-bot: Verified
diff --git a/includes/DefaultSettings.php b/includes/DefaultSettings.php
index 9221784..173605c 100644
--- a/includes/DefaultSettings.php
+++ b/includes/DefaultSettings.php
@@ -3497,8 +3497,9 @@
$wgAllowDisplayTitle = true;
/**
- * For consistency, restrict DISPLAYTITLE to titles that normalize to the same
- * canonical DB key.
+ * For consistency, restrict DISPLAYTITLE to text that normalizes to the same
+ * canonical DB key. Also disallow some inline CSS rules like display: none;
+ * which can cause the text to be hidden or unselectable.
*/
$wgRestrictDisplayTitle = true;
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index ed01235..b4a1c62 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -813,9 +813,10 @@
/**
* Pick apart some CSS and check it for forbidden or unsafe structures.
* Returns a sanitized string. This sanitized string will have
-* character references and escape sequences decoded, and comments
-* stripped. If the input is just too evil, only a comment complaining
-* about evilness will be returned.
+* character references and escape sequences decoded and comments
+* stripped (unless it is itself one valid comment, in which case the
value
+* will be passed through). If the input is just too evil, only a
comment
+* complaining about evilness will be returned.
*
* Currently URL references, 'expression', 'tps' are forbidden.
*
@@ -856,19 +857,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, '/*' );
+
[MediaWiki-commits] [Gerrit] displaytitle: reject some CSS if $wgRestrictDisplayTitle set - change (mediawiki/core)
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: Ie162535b6bcbebce4ee69f6dcc1957c3c672
---
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
