From 30b7f71fc9a2bc7cba8d7363bffa013fa74c7f1b Mon Sep 17 00:00:00 2001 From: Matthias Mullie Date: Mon, 10 Mar 2025 09:21:48 +0100 Subject: [PATCH] SECURITY: Fix XSS vulnerability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Attributes prefixed data-mw- are covered by Sanitizer.php and should be safe to use. Others are not safe, as they can also be used in Wikitext. For data-(mw-)property and data-(mw-)statements, fall back to the old attribute value for cached cases where the new ones are missing. This is hopefully relatively safe – Wikitext with custom values for these attributes may be able to cause confusing on-page behavior, but hopefully not code execution. To further safeguard this, make the selector more precise – this should hopefully cut out most user-generated content (though discussion on the task is inconclusive, as of this writing, whether it’s fully enough or not). For data-(mw-)formatvalue, don’t fall back to the old values. Values of this attribute are directly used as HTML and are thus highly unsafe; if we can’t use them, the wbformatvalue cache should just fall back to getting them from the API, which is slightly less efficient but safe. Bug: T387691 Change-Id: Ie969a8cfeab0d4457417773fa884e271968e5657 --- resources/filepage/StatementPanel.js | 4 ++-- resources/filepage/init.js | 17 +++++++++++++---- .../inputs/EntityAutocompleteInputWidget.js | 2 +- src/View/MediaInfoEntityStatementsView.php | 11 +++-------- ...ityAutocompleteInputWidgetLabel.mustache+dom | 2 +- 5 files changed, 20 insertions(+), 16 deletions(-) diff --git a/resources/filepage/StatementPanel.js b/resources/filepage/StatementPanel.js index 4bbc1e6d..40dc1cca 100644 --- a/resources/filepage/StatementPanel.js +++ b/resources/filepage/StatementPanel.js @@ -36,8 +36,8 @@ const StatementPanel = function StatementPanelConstructor( config ) { // Mixin constructors OO.ui.mixin.PendingElement.call( this, this.config ); - if ( this.$element.attr( 'data-formatvalue' ) ) { - this.populateFormatValueCache( JSON.parse( this.$element.attr( 'data-formatvalue' ) || '{}' ) ); + if ( this.$element.attr( 'data-mw-formatvalue' ) ) { + this.populateFormatValueCache( JSON.parse( this.$element.attr( 'data-mw-formatvalue' ) || '{}' ) ); } this.licenseDialogWidget = new LicenseDialogWidget(); diff --git a/resources/filepage/init.js b/resources/filepage/init.js index 9bcce6c8..236523ed 100644 --- a/resources/filepage/init.js +++ b/resources/filepage/init.js @@ -174,7 +174,7 @@ mw.hook( 'wikipage.content' ).add( ( $content ) => { const linkNoticeWidget = new LinkNoticeWidget(); const protectionMsgWidget = new ProtectionMsgWidget(); - const $statements = $content.find( '.wbmi-entityview-statementsGroup' ); + const $statements = $content.find( '.wbmi-structured-data-header ~ .wbmi-entityview-statementsGroup' ); const existingProperties = defaultProperties.concat( Object.keys( mediaInfoEntity.statements || {} ) ); const deserializer = new StatementListDeserializer(); @@ -198,7 +198,7 @@ $content.find( '.wbmi-tabs-container' ).first().before( protectionMsgWidget.$element ); const captionsPanel = createCaptionsPanel(); // eslint-disable-next-line no-jquery/no-global-selector - $( '.wbmi-entityview-captionsPanel' ).replaceWith( captionsPanel.$element ); + $( '.wbmi-captions-header ~ .wbmi-entityview-captionsPanel' ).replaceWith( captionsPanel.$element ); // Add link notice widget and add property button if they don't exist. if ( $statements.first().hasClass( 'wbmi-entityview-statementsGroup' ) ) { @@ -212,8 +212,17 @@ // Set up existing statement panels const existingStatementPanels = $statements.get().map( ( element ) => { const $statement = $( element ), - propId = $statement.data( 'property' ), - statementsJson = JSON.parse( $statement.attr( 'data-statements' ) || '[]' ), + propId = $statement.data( 'mw-property' ) || + // Fallback for when this attribute was named differently + // @see https://phabricator.wikimedia.org/T387691 + $statement.data( 'property' ), + statementsJson = JSON.parse( + $statement.attr( 'data-mw-statements' ) || + // Fallback for when this attribute was named differently + // @see https://phabricator.wikimedia.org/T387691 + $statement.attr( 'data-statements' ) || + '[]' + ), data = deserializer.deserialize( statementsJson ), statementPanel = createStatementsPanel( $statement, diff --git a/resources/statements/inputs/EntityAutocompleteInputWidget.js b/resources/statements/inputs/EntityAutocompleteInputWidget.js index bc5bbce0..211b6d8b 100644 --- a/resources/statements/inputs/EntityAutocompleteInputWidget.js +++ b/resources/statements/inputs/EntityAutocompleteInputWidget.js @@ -211,7 +211,7 @@ EntityAutocompleteInputWidget.prototype.onMousedown = function ( e ) { // window.open. This is a response to a mousedown event so it shouldn't // trigger any popup blockers in modern browsers. For browsers set to // prefer new tabs over new windows, this will open in a new tab. - window.open( e.currentTarget.dataset.url, '_blank' ); + window.open( e.currentTarget.dataset.mwUrl, '_blank' ); } }; diff --git a/src/View/MediaInfoEntityStatementsView.php b/src/View/MediaInfoEntityStatementsView.php index 24a3e224..f24322b4 100644 --- a/src/View/MediaInfoEntityStatementsView.php +++ b/src/View/MediaInfoEntityStatementsView.php @@ -261,9 +261,9 @@ class MediaInfoEntityStatementsView { ] ); $panel->setAttributes( [ - 'data-property' => $propertyIdString, - 'data-statements' => json_encode( $serializedStatements ), - 'data-formatvalue' => json_encode( $formatValueCache ), + 'data-mw-property' => $propertyIdString, + 'data-mw-statements' => json_encode( $serializedStatements ), + 'data-mw-formatvalue' => json_encode( $formatValueCache ), ] ); return $panel; @@ -350,11 +350,6 @@ class MediaInfoEntityStatementsView { $statementDiv = new Tag( 'div' ); $statementDiv->addClasses( [ 'wbmi-item-container' ] ); - $guid = $statement->getGuid(); - if ( $guid !== null ) { - $statementDiv->setAttributes( [ 'data-guid' => $guid ] ); - } - $mainSnakDiv = new Tag( 'div' ); $mainSnakDiv->addClasses( [ 'wbmi-entity-header' ] ); $mainSnakDiv->appendContent( diff --git a/templates/statements/inputs/EntityAutocompleteInputWidgetLabel.mustache+dom b/templates/statements/inputs/EntityAutocompleteInputWidgetLabel.mustache+dom index 7320a630..77957477 100644 --- a/templates/statements/inputs/EntityAutocompleteInputWidgetLabel.mustache+dom +++ b/templates/statements/inputs/EntityAutocompleteInputWidgetLabel.mustache+dom @@ -1,4 +1,4 @@ - + {{label}} -- 2.34.1