T110143b-REL1_23.patch
csteipp (Chris Steipp)
Actions

Authored By

	• csteipp
	Apr 25 2016, 11:04 PM

Size

4 KB

Referenced Files

None

Subscribers

None

T110143b-REL1_23.patch
View Options

	From 95ef7f76968592f1176a1a38d2641ebfd4b874fd Mon Sep 17 00:00:00 2001
	From: csteipp <csteipp@wikimedia.org>
	Date: Mon, 25 Apr 2016 15:25:18 -0700
	Subject: [PATCH] SECURITY: Include quote characters in strip markers so esc in
	attr

	Strip markers get substituted for general html, which means the
	substitution text general does not escape quote characters. If
	someone can convince MW to put a strip marker in an attribute,
	you can get around escaping requirements that way. This patch
	adds the characters `"' to the strip marker text. At least one
	of these characters should be escaped inside attributes (regardless
	of what quote character you use for attributes), thus normal html
	escaping will deactivate the strip markers, preventing the
	vulnrability.

	This will break any extension that escapes input with htmlspecialchars,
	to add to html/half parsed html output, but assumes that strip markers
	are unmangled. I don't think its very common to do this. The primary
	example I found was some core usages of Xml::escapeTagsOnly(). (And
	even in that case, it only affected the corner case of being called
	via {{#tag:..}})

	Bug: T110143
	Change-Id: If887065e12026530f36e5f35dd7ab0831d313561
	---
	includes/parser/CoreTagHooks.php \| 24 +++++++++++++++++++-----
	includes/parser/Parser.php \| 5 +++--
	tests/parser/parserTests.txt \| 9 +++++++++
	3 files changed, 31 insertions(+), 7 deletions(-)

	diff --git a/includes/parser/CoreTagHooks.php b/includes/parser/CoreTagHooks.php
	index cbc060a..71f3faa 100644
	--- a/includes/parser/CoreTagHooks.php
	+++ b/includes/parser/CoreTagHooks.php
	@@ -55,9 +55,14 @@ class CoreTagHooks {
	$content = StringUtils::delimiterReplace( '<nowiki>', '</nowiki>', '$1', $text, 'i' );

	$attribs = Sanitizer::validateTagAttributes( $attribs, 'pre' );
	- return Xml::openElement( 'pre', $attribs ) .
	- Xml::escapeTagsOnly( $content ) .
	- '</pre>';
	+ // We need to let both '"' and '&' through,
	+ // for strip markers and entities respectively.
	+ $content = str_replace(
	+ array( '>', '<' ),
	+ array( '>', '<' ),
	+ $content
	+ );
	+ return Html::rawElement( 'pre', $attribs, $content );
	}

	/**
	@@ -97,8 +102,17 @@ class CoreTagHooks {
	* @return array
	*/
	static function nowiki( $content, $attributes, $parser ) {
	- $content = strtr( $content, array( '-{' => '-{', '}-' => '}-' ) );
	- return array( Xml::escapeTagsOnly( $content ), 'markerType' => 'nowiki' );
	+ $content = strtr( $content, array(
	+ // lang converter
	+ '-{' => '-{',
	+ '}-' => '}-',
	+ // html tags
	+ '<' => '<',
	+ '>' => '>'
	+ // Note: Both '"' and '&' are not converted.
	+ // This allows strip markers and entities through.
	+ ) );
	+ return array( $content, 'markerType' => 'nowiki' );
	}

	/**
	diff --git a/includes/parser/Parser.php b/includes/parser/Parser.php
	index 2d9078d..74ef974 100644
	--- a/includes/parser/Parser.php
	+++ b/includes/parser/Parser.php
	@@ -113,7 +113,8 @@ class Parser {
	const OT_PLAIN = 4; # like extractSections() - portions of the original are returned unchanged.

	# Marker Suffix needs to be accessible staticly.
	- const MARKER_SUFFIX = "-QINU\x7f";
	+ # Must have a character that needs escaping in attributes (since 1.25.6)
	+ const MARKER_SUFFIX = "-QINU`\"'\x7f";

	# Markers used for wrapping the table of contents
	const TOC_START = '<mw:toc>';
	@@ -312,7 +313,7 @@ class Parser {
	* Must not consist of all title characters, or else it will change
	* the behavior of <nowiki> in a link.
	*/
	- $this->mUniqPrefix = "\x7fUNIQ" . self::getRandomString();
	+ $this->mUniqPrefix = "\x7f'\"`UNIQ" . self::getRandomString();
	$this->mStripState = new StripState( $this->mUniqPrefix );

	# Clear these on every parse, bug 4549
	diff --git a/tests/parser/parserTests.txt b/tests/parser/parserTests.txt
	index c3e972e..684cb87 100644
	--- a/tests/parser/parserTests.txt
	+++ b/tests/parser/parserTests.txt
	@@ -1537,6 +1537,15 @@ Entities inside <pre>
	!! end

	!! test
	+<nowiki> inside of #tag:pre
	+!! wikitext
	+{{#tag:pre\|Foo <nowiki>→bar</nowiki>}}
	+!! html
	+<pre>Foo →bar</pre>
	+
	+!! end
	+
	+!! test
	<nowiki> and <pre> preference (first one wins)
	!! wikitext
	<pre>
	--
	2.6.6

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 3678984
Default Alt Text: T110143b-REL1_23.patch (4 KB)

Attached To	Mode
T110143: strip markers can be used to get around html attribute escaping in (many?) parser tags	Attached	Detach File
T124940: MediaWiki 1.26.3 security release	Attached	Detach File

T110143b-REL1_23.patchcsteipp (Chris Steipp)Actions

T110143b-REL1_23.patchView Options

File Metadata

Event Timeline

T110143b-REL1_23.patch
csteipp (Chris Steipp)
Actions

T110143b-REL1_23.patch
View Options