Add missing HTML escaping to all existing page preview types

Including tests for all situations.

I believe it is impossible or extremely hard to actually abuse any of
these places. All these data are not extracted from the current page, but
delivered either by MediaWiki's api.php or a RESTful endpoint, as
configured via $wgPopupsGateway and $wgPopupsRestGatewayEndpoint. A
possible attacker would need to write it's own endpoint (which must either
run on the same server or somehow ignore the CSRF token), and set the
value of mw.config.values.wgPopupsRestGatewayEndpoint on the client to
this endpoint – which requires just *another* attack vector to be able to
do this.

It's "the right thing"(tm) to escape all this anyway.

I found two possibly relevant security reviews of this extension, T88171
and T129177, resolved in 2015 and 2016.

Bug: T88171
Bug: T129177
Bug: T214754
Bug: T214971
Change-Id: I1d118c9ccaea434a253a772d18139b9b077118ab

src/ui/templates/pagePreview/pagePreview.js

@@ -3,6 +3,7 @@
  */
 
 import { renderPopup } from '../popup/popup';
+import { escapeHTML } from '../templateUtil';
 
 /**
  * @param {ext.popups.PreviewModel} model
@@ -10,9 +11,13 @@ import { renderPopup } from '../popup/popup';
  * @return {string} HTML string.
  */
 export function renderPagePreview(
-	{ url, type, languageCode, languageDirection }, hasThumbnail
+	model, hasThumbnail
 ) {
-	return renderPopup( type,
+	const url = escapeHTML( model.url ),
+		languageCode = escapeHTML( model.languageCode ),
+		languageDirection = escapeHTML( model.languageDirection );
+
+	return renderPopup( model.type,
 		`
 			${ hasThumbnail ? `<a href='${ url }' class='mwe-popups-discreet'></a>` : '' }
 			<a dir='${ languageDirection }' lang='${ languageCode }' class='mwe-popups-extract' href='${ url }'></a>

src/ui/templates/popup/popup.js

@@ -2,12 +2,16 @@
  * @module popup
  */
 
+import { escapeHTML } from '../templateUtil';
+
 /**
  * @param {ext.popups.previewTypes} type
  * @param {string} html HTML string.
  * @return {string} HTML string.
  */
 export function renderPopup( type, html ) {
+	type = escapeHTML( type );
+
 	return `
 	<div class='mwe-popups mwe-popups-type-${ type }' role='tooltip' aria-hidden>
 		<div class='mwe-popups-container'>${ html }</div>

src/ui/templates/preview/preview.js

@@ -13,12 +13,15 @@ import { escapeHTML } from '../templateUtil';
  * @return {string} HTML string.
  */
 export function renderPreview(
-	{ title, url, type }, showTitle, extractMsg, linkMsg
+	model, showTitle, extractMsg, linkMsg
 ) {
-	title = escapeHTML( title );
+	const title = escapeHTML( model.title ),
+		url = escapeHTML( model.url ),
+		type = escapeHTML( model.type );
 	extractMsg = escapeHTML( extractMsg );
 	linkMsg = escapeHTML( linkMsg );
-	return renderPopup( type,
+
+	return renderPopup( model.type,
 		`
 			<div class='mw-ui-icon mw-ui-icon-element mw-ui-icon-preview-${ type }'></div>
 			${ showTitle ? `<strong class='mwe-popups-title'>${ title }</strong>` : '' }

tests/node-qunit/ui/renderer.test.js

@@ -122,9 +122,9 @@ QUnit.test( 'createPointerMasks', ( assert ) => {
 QUnit.test( 'createPagePreview', ( assert ) => {
 	const model = {
 			title: 'Test',
-			url: 'https://en.wikipedia.org/wiki/Test',
-			languageCode: 'en',
-			languageDirection: 'ltr',
+			url: 'https://en.wikipedia.org/wiki/Test <"\'>',
+			languageCode: 'en <"\'>',
+			languageDirection: 'ltr <"\'>',
 			extract: 'This is a test page.',
 			type: previewTypes.TYPE_PAGE,
 			thumbnail: {
@@ -151,12 +151,28 @@ QUnit.test( 'createPagePreview', ( assert ) => {
 		'This is a test page.',
 		'Preview extract is correct.'
 	);
+
+	assert.strictEqual(
+		preview.el.find( '.mwe-popups-extract' ).attr( 'href' ),
+		'https://en.wikipedia.org/wiki/Test <"\'>',
+		'URL is safely espaced'
+	);
+	assert.strictEqual(
+		preview.el.find( '.mwe-popups-extract' ).attr( 'lang' ),
+		'en <"\'>',
+		'Language code is safely espaced'
+	);
+	assert.strictEqual(
+		preview.el.find( '.mwe-popups-extract' ).attr( 'dir' ),
+		'ltr <"\'>',
+		'Language direction is safely espaced'
+	);
 } );
 
 QUnit.test( 'createEmptyPreview(model)', ( assert ) => {
 	const model = {
 			title: 'Test',
-			url: 'https://en.wikipedia.org/wiki/Test',
+			url: 'https://en.wikipedia.org/wiki/Test <"\'>',
 			languageCode: 'en',
 			languageDirection: 'ltr',
 			extract: 'This is a test page.',
@@ -196,6 +212,11 @@ QUnit.test( 'createEmptyPreview(model)', ( assert ) => {
 		MSG_GO_TO_PAGE,
 		'Empty preview link text is correct.'
 	);
+	assert.strictEqual(
+		emptyPreview.el.find( '.mwe-popups-read-link' ).attr( 'href' ),
+		'https://en.wikipedia.org/wiki/Test <"\'>',
+		'URL is safely espaced'
+	);
 } );
 
 QUnit.test( 'createEmptyPreview(null model)', ( assert ) => {
@@ -231,10 +252,34 @@ QUnit.test( 'createEmptyPreview(null model)', ( assert ) => {
 	);
 } );
 
+QUnit.test( 'createPreviewWithType(model with unknown type)', ( assert ) => {
+	const model = {
+			url: '/wiki/Unknown <"\'>',
+			type: 'unknown <"\'>'
+		},
+		emptyPreview = renderer.createPreviewWithType( model );
+
+	assert.strictEqual(
+		emptyPreview.el.find( '.mwe-popups-extract' ).attr( 'href' ),
+		'/wiki/Unknown <"\'>',
+		'URL is safely espaced'
+	);
+	assert.strictEqual(
+		emptyPreview.el.attr( 'class' ),
+		'mwe-popups mwe-popups-type-unknown <"\'>',
+		'Popup type is safely espaced'
+	);
+	assert.strictEqual(
+		emptyPreview.el.find( '.mw-ui-icon' ).attr( 'class' ),
+		'mw-ui-icon mw-ui-icon-element mw-ui-icon-preview-unknown <"\'>',
+		'Icon type is safely espaced'
+	);
+} );
+
 QUnit.test( 'createDisambiguationPreview(model)', ( assert ) => {
 	const model = {
 			title: 'Barack (disambiguation)',
-			url: 'url/Barack (disambiguation)',
+			url: 'url/Barack (disambiguation) <"\'>',
 			languageCode: 'en',
 			languageDirection: 'ltr',
 			extract: 'Barack Hussein Obama II born August 4, 1961) ...',
@@ -269,6 +314,11 @@ QUnit.test( 'createDisambiguationPreview(model)', ( assert ) => {
 		MSG_DISAMBIGUATION_LINK,
 		'Preview link text is correct.'
 	);
+	assert.strictEqual(
+		preview.el.find( '.mwe-popups-read-link' ).attr( 'href' ),
+		'url/Barack (disambiguation) <"\'>',
+		'URL is safely espaced'
+	);
 } );
 
 QUnit.test( 'createReferencePreview(model)', ( assert ) => {
@@ -298,7 +348,8 @@ QUnit.test( 'createReferencePreview(model)', ( assert ) => {
 	);
 	assert.strictEqual(
 		preview.el.find( '.mwe-popups-read-link' ).attr( 'href' ),
-		'#custom_id <"\'>'
+		'#custom_id <"\'>',
+		'URL is safely espaced'
 	);
 } );