From 4e28c50b934a7f9fdc18e78b27104a98d75365a4 Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
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(
+							$( '<span>' ),
+							'[' + 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' ),
+			'<span>[javascript:alert(1) foo]</span>',
+			'illegal-url: \'parse\' format'
+		);
+	} );
+	QUnit.test( 'Do not allow arbitrary style', 3, function ( assert ) {
+		mw.messages.set( 'illegal-style', '<span style="background-image:url( http://example.com )">bar</span>' );
+
+		this.suppressWarnings();
+
+		assert.equal(
+			formatParse( 'illegal-url' ),
+			'&lt;span style="background-image:url( http://example.com )"&gt;bar&lt;/span&gt;',
+			'illegal-style: \'parse\' format'
+		);
+	} );
+
 
 }( mediaWiki, jQuery ) );
-- 
2.0.1

