From 7c769ae81d90a982bac7a3eee556e83ceb01ad52 Mon Sep 17 00:00:00 2001 From: BlankEclair Date: Mon, 9 Dec 2024 21:29:50 +1100 Subject: [PATCH] SECURITY: Fix several XSSes Bug: T381753 --- api/ApiViewActivityArticleFeedbackv5.php | 2 +- .../jquery.articleFeedbackv5.js | 74 +++++++++---------- .../jquery.articleFeedbackv5.special.js | 2 +- 3 files changed, 35 insertions(+), 43 deletions(-) diff --git a/api/ApiViewActivityArticleFeedbackv5.php b/api/ApiViewActivityArticleFeedbackv5.php index 72c44b75..7e79b661 100644 --- a/api/ApiViewActivityArticleFeedbackv5.php +++ b/api/ApiViewActivityArticleFeedbackv5.php @@ -107,7 +107,7 @@ class ApiViewActivityArticleFeedbackv5 extends ApiQueryBase { ->params( $feedback->aft_id ) ->rawParams( ArticleFeedbackv5Utils::getUserLink( $feedback->aft_user, $feedback->aft_user_text ) ) ->params( $feedback->aft_user_text ) // username or ip - ->text() + ->escaped() ) . Html::element( 'div', diff --git a/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js b/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js index 25ebf688..348960d2 100644 --- a/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js +++ b/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.js @@ -707,7 +707,7 @@ $( '.articleFeedbackv5-arrow-back' ).remove(); // reset regular title - $( '.articleFeedbackv5-title' ).html( $.articleFeedbackv5.currentBucket().getTitle() ); + $( '.articleFeedbackv5-title' ).text( $.articleFeedbackv5.currentBucket().getTitle() ); }, // }}} @@ -1319,10 +1319,10 @@ // and it needs to show up if the site subhead (e.g., "From Wikipedia, the free // encyclopedia") is not visible for any reason. if ( $( '#siteSub' ).filter( ':visible' ).length ) { - $link.prepend( '   ' + mw.msg( 'pipe-separator' ) + '   ' ); + $link.prepend( '   ', mw.message( 'pipe-separator' ).escaped(), '   ' ); $( '#siteSub' ).append( $link ); } else if ( $( 'h1.pagetitle + p.subtitle' ).filter( ':visible' ).length ) { - $link.prepend( '   ' + mw.msg( 'pipe-separator' ) + '   ' ); + $link.prepend( '   ', mw.message( 'pipe-separator' ).escaped(), '   ' ); $( 'h1.pagetitle + p.subtitle' ).append( $link ); } else if ( $( '#mw_contentholder .mw-topboxes' ).length ) { $( '#mw_contentholder .mw-topboxes' ).after( $link ); @@ -2038,47 +2038,39 @@ * @return {string} the html */ $.articleFeedbackv5.buildLink = function ( fulltext ) { - var full, args, i, sub, replacement; - - full = mw.html.escape( mw.msg( fulltext ) ); - args = arguments; - return full.replace( /\$(\d+)/g, function ( str, number ) { - i = parseInt( number, 10 ); - sub = args[ i ]; - replacement = ''; - if ( sub.tag === 'quotes' ) { - replacement = '"' + mw.msg( sub.text ) + '"'; - } else { - replacement = mw.html.element( - 'tag' in sub ? sub.tag : 'a', - $.articleFeedbackv5.attribs( sub ), - mw.msg( sub.text ) - ).toString(); - } - return replacement; - } ); - }; + var parameters, i, sub, tag, text, $element, key; - // }}} - // {{{ attribs + parameters = []; + for ( i = 1; i < arguments.length; i++ ) { + sub = arguments[ i ]; + tag = sub.tag; + text = mw.msg( sub.text ); - /** - * Utility method: Set up the attributes for a link (works with - * buildLink()) - * - * @param {Object} link the first link, as { href: '#', text: 'click here'. - * other-attrib: 'whatever'} - * @return {Object} the attributes - */ - $.articleFeedbackv5.attribs = function ( link ) { - var k, attr = {}; + delete sub.tag; + delete sub.text; + + if ( tag === 'quotes' ) { + parameters.push( '"' + text + '"' ); + } else { + // We don't do the $( html, attributes ) shorthand syntax since that can + // call functions, and while it would be weird to define an attribute with + // the same name as a jQuery function, the previous code would have treated + // it like a regular attribute, so it's being replicated here. + $element = $( '<' + tag + '/>' ); + for ( key in sub ) { + $element.attr( key, sub[ key ] ); + } + $element.text( text ); - for ( k in link ) { - if ( k !== 'text' && k !== 'tag' ) { - attr[ k ] = link[ k ]; + parameters.push( $element ); } } - return attr; + + // This really needs to be Message.parse()--the documentation says "DOM/jQuery parameters + // can be used to achieve the equivalent of rawParams()", but this seemingly only works + // for Message.parse(), not Message.escaped(). Debug this if you want to, but this is good + // enough + return mw.message( fulltext, ...parameters ).parse(); }; // }}} @@ -2272,7 +2264,7 @@ // Set the title if ( 'getTitle' in bucket ) { - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-title' ).html( bucket.getTitle() ); + $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-title' ).text( bucket.getTitle() ); } // Link to help is dependent on the group the user belongs to @@ -2815,7 +2807,7 @@ * @param {string} msg the error message */ $.articleFeedbackv5.markTopError = function ( msg ) { - $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-top-error' ).html( msg ); + $.articleFeedbackv5.$holder.find( '.articleFeedbackv5-top-error' ).text( msg ); }; // }}} diff --git a/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js b/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js index fd4ea895..32d01c07 100644 --- a/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js +++ b/modules/jquery.articleFeedbackv5/jquery.articleFeedbackv5.special.js @@ -1706,7 +1706,7 @@
\
\

' + - mw.msg( 'articlefeedbackv5-activity-pane-header' ) + + mw.message( 'articlefeedbackv5-activity-pane-header' ).escaped() + '

\ \
\ -- 2.47.1