Gergő Tisza has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/359047 )
Change subject: Fix changes list data attribute sanitizing
......................................................................
Fix changes list data attribute sanitizing
The attribute sanitizer should disallow reserved attributes,
not keep only those.
Bug: T167922
Change-Id: Ic24400382a9dcbb990e12dfddae4ab7db14553cc
---
M includes/Sanitizer.php
M includes/actions/HistoryAction.php
M includes/changes/EnhancedChangesList.php
M includes/changes/OldChangesList.php
M includes/logging/LogEventsList.php
M includes/specials/SpecialNewpages.php
M includes/specials/pagers/ContribsPager.php
M includes/specials/pagers/DeletedContribsPager.php
8 files changed, 12 insertions(+), 12 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/mediawiki/core
refs/changes/47/359047/1
diff --git a/includes/Sanitizer.php b/includes/Sanitizer.php
index 8920e92..224dfae1 100644
--- a/includes/Sanitizer.php
+++ b/includes/Sanitizer.php
@@ -787,7 +787,7 @@
# colons.
if ( !preg_match( '/^data-[^:]*$/i', $attribute )
&& !isset( $whitelist[$attribute] )
- || self::isReservedDataAttribute( $attribute )
+ || !self::isSafeDataAttribute( $attribute )
) {
continue;
}
@@ -856,13 +856,13 @@
}
/**
- * Given an attribute name, checks whether it is a reserved data
attribute
+ * Given an attribute name, checks that it is not a reserved data
attribute
* (such as data-mw-foo) which is unavailable to user-generated HTML so
MediaWiki
* core and extension code can safely use it to communicate with
frontend code.
* @param string $attr Attribute name.
* @return bool
*/
- public static function isReservedDataAttribute( $attr ) {
+ public static function isSafeDataAttribute( $attr ) {
// data-ooui is reserved for ooui.
// data-mw and data-parsoid are reserved for parsoid.
// data-mw-<name here> is reserved for extensions (or core) if
@@ -870,7 +870,7 @@
// sure that it isn't coming from an untrusted user.
// We ignore the possibility of namespaces since user-generated
HTML
// can't use them anymore.
- return (bool)preg_match( '/^data-(ooui|mw|parsoid)/i', $attr );
+ return !(bool)preg_match( '/^data-(ooui|mw|parsoid)/i', $attr );
}
/**
diff --git a/includes/actions/HistoryAction.php
b/includes/actions/HistoryAction.php
index 7460340..dbeaf73 100644
--- a/includes/actions/HistoryAction.php
+++ b/includes/actions/HistoryAction.php
@@ -783,7 +783,7 @@
$attribs = [ 'data-mw-revid' => $rev->getId() ];
Hooks::run( 'PageHistoryLineEnding', [ $this, &$row, &$s,
&$classes, &$attribs ] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
if ( $classes ) {
$attribs['class'] = implode( ' ', $classes );
diff --git a/includes/changes/EnhancedChangesList.php
b/includes/changes/EnhancedChangesList.php
index 03f63f6..3436e06 100644
--- a/includes/changes/EnhancedChangesList.php
+++ b/includes/changes/EnhancedChangesList.php
@@ -456,7 +456,7 @@
// skip entry if hook aborted it
return [];
}
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
$lineParams['recentChangesFlagsRaw'] = [];
if ( isset( $data['recentChangesFlags'] ) ) {
@@ -686,7 +686,7 @@
}
$attribs = $data['attribs'];
unset( $data['attribs'] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
$line = Html::openElement( 'table', $attribs ) .
Html::openElement( 'tr' );
$line .= '<td class="mw-enhanced-rc"><span
class="mw-enhancedchanges-arrow-space"></span>';
diff --git a/includes/changes/OldChangesList.php
b/includes/changes/OldChangesList.php
index 2a53d66..da963c9 100644
--- a/includes/changes/OldChangesList.php
+++ b/includes/changes/OldChangesList.php
@@ -59,7 +59,7 @@
) {
return false;
}
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
$dateheader = ''; // $html now contains only <li>...</li>, for
hooks' convenience.
$this->insertDateHeader( $dateheader,
$rc->mAttribs['rc_timestamp'] );
diff --git a/includes/logging/LogEventsList.php
b/includes/logging/LogEventsList.php
index c5501cb..1fde3e1 100644
--- a/includes/logging/LogEventsList.php
+++ b/includes/logging/LogEventsList.php
@@ -398,7 +398,7 @@
// Let extensions add data
Hooks::run( 'LogEventsListLineEnding', [ $this, &$ret, $entry,
&$classes, &$attribs ] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
$attribs['class'] = implode( ' ', $classes );
return Html::rawElement( 'li', $attribs, $ret ) . "\n";
diff --git a/includes/specials/SpecialNewpages.php
b/includes/specials/SpecialNewpages.php
index 83482f6..4954754 100644
--- a/includes/specials/SpecialNewpages.php
+++ b/includes/specials/SpecialNewpages.php
@@ -387,7 +387,7 @@
// Let extensions add data
Hooks::run( 'NewPagesLineEnding', [ $this, &$ret, $result,
&$classes, &$attribs ] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
if ( count( $classes ) ) {
$attribs['class'] = implode( ' ', $classes );
diff --git a/includes/specials/pagers/ContribsPager.php
b/includes/specials/pagers/ContribsPager.php
index 6bd7eb0..91c961c 100644
--- a/includes/specials/pagers/ContribsPager.php
+++ b/includes/specials/pagers/ContribsPager.php
@@ -536,7 +536,7 @@
// Let extensions add data
Hooks::run( 'ContributionsLineEnding', [ $this, &$ret, $row,
&$classes, &$attribs ] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
// TODO: Handle exceptions in the catch block above. Do any
extensions rely on
// receiving empty rows?
diff --git a/includes/specials/pagers/DeletedContribsPager.php
b/includes/specials/pagers/DeletedContribsPager.php
index 43d7ad4..e456ed4 100644
--- a/includes/specials/pagers/DeletedContribsPager.php
+++ b/includes/specials/pagers/DeletedContribsPager.php
@@ -220,7 +220,7 @@
// Let extensions add data
Hooks::run( 'DeletedContributionsLineEnding', [ $this, &$ret,
$row, &$classes, &$attribs ] );
- $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isReservedDataAttribute' ] );
+ $attribs = wfArrayFilterByKey( $attribs, [ Sanitizer::class,
'isSafeDataAttribute' ] );
if ( $classes === [] && $attribs === [] && $ret === '' ) {
wfDebug( "Dropping Special:DeletedContribution row that
could not be formatted\n" );
--
To view, visit https://gerrit.wikimedia.org/r/359047
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic24400382a9dcbb990e12dfddae4ab7db14553cc
Gerrit-PatchSet: 1
Gerrit-Project: mediawiki/core
Gerrit-Branch: master
Gerrit-Owner: Gergő Tisza <gti...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
