From 4e28c50b934a7f9fdc18e78b27104a98d75365a4 Mon Sep 17 00:00:00 2001 From: Brian Wolff Date: Tue, 27 Oct 2015 01:39:34 -0600 Subject: [PATCH] Make jqueryMsg parser sanitize urls and style Previously you could leverage the style attribute, and external links to execute javascript. Change-Id: I6f15ece1db136369e06dfeee34d1a0c5bc03e32b --- resources/src/mediawiki/mediawiki.jqueryMsg.js | 21 ++++++++++++++++++-- .../mediawiki/mediawiki.jqueryMsg.test.js | 23 ++++++++++++++++++++++ 2 files changed, 42 insertions(+), 2 deletions(-) diff --git a/resources/src/mediawiki/mediawiki.jqueryMsg.js b/resources/src/mediawiki/mediawiki.jqueryMsg.js index de63a18..0fc994d 100644 --- a/resources/src/mediawiki/mediawiki.jqueryMsg.js +++ b/resources/src/mediawiki/mediawiki.jqueryMsg.js @@ -699,6 +699,12 @@ attributeName = attributes[ i ]; if ( $.inArray( attributeName, settings.allowedHtmlCommonAttributes ) === -1 && $.inArray( attributeName, settings.allowedHtmlAttributesByElement[ startTagName ] || [] ) === -1 ) { + return false; + } + if ( attributeName === 'style' && attributes[ i + 1 ].search( + /[\000-\010\013\016-\037\177]|expression|filter\s*:|accelerator\s*:|-o-link\s*:|-o-link-source\s*:|-o-replace\s*:|url\s*\(|image\s*\(|image-set\s*\(/i + ) !== -1 ) { + mw.log( "HTML tag removed from message due to dangerous style attribute" ); return false; } } @@ -1104,7 +1110,8 @@ extlink: function ( nodes ) { var $el, arg = nodes[ 0 ], - contents = nodes[ 1 ]; + contents = nodes[ 1 ], + target; if ( arg instanceof jQuery && !arg.hasClass( 'mediaWiki_htmlEmitter' ) ) { $el = arg; } else { @@ -1116,7 +1123,17 @@ } ) .click( arg ); } else { - $el.attr( 'href', textify( arg ) ); + target = textify( arg ); + if ( target.search( new RegExp( '^(' + mw.config.get( 'wgUrlProtocols' ) + ')' ) ) !== -1 ) { + $el.attr( 'href', target ); + } else { + // What is proper thing to do here + mw.log( "External link in message had illegal target " + target ); + return appendWithoutParsing( + $( '' ), + '[' + target + ' ' + textify( contents ) + ']' + ); + } } } return appendWithoutParsing( $el, contents ); diff --git a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js index 4f273bc..6581c8c 100644 --- a/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js +++ b/tests/qunit/suites/resources/mediawiki/mediawiki.jqueryMsg.test.js @@ -1077,5 +1077,28 @@ assert.equal( logSpy.callCount, 2, 'mw.log.warn calls' ); } ); + QUnit.test( 'Do not allow js urls', 3, function ( assert ) { + mw.messages.set( 'illegal-url', '[javascript:alert(1) foo]' ); + + this.suppressWarnings(); + + assert.equal( + formatParse( 'illegal-url' ), + '[javascript:alert(1) foo]', + 'illegal-url: \'parse\' format' + ); + } ); + QUnit.test( 'Do not allow arbitrary style', 3, function ( assert ) { + mw.messages.set( 'illegal-style', 'bar' ); + + this.suppressWarnings(); + + assert.equal( + formatParse( 'illegal-url' ), + '<span style="background-image:url( http://example.com )">bar</span>', + 'illegal-style: \'parse\' format' + ); + } ); + }( mediaWiki, jQuery ) ); -- 2.0.1