T110143b-REL1_25.patch
csteipp (Chris Steipp)
Actions

Authored By

	• csteipp
	Apr 25 2016, 11:05 PM

Size

4 KB

Referenced Files

None

Subscribers

None

T110143b-REL1_25.patch
View Options

	From 05a50de226edfe7a6fd5f6802e91da6c61ed0c28 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 9755ea9..3f4f54a 100644
	--- a/includes/parser/CoreTagHooks.php
	+++ b/includes/parser/CoreTagHooks.php
	@@ -56,9 +56,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 );
	}

	/**
	@@ -98,8 +103,17 @@ class CoreTagHooks {
	* @return array
	*/
	public 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 ace63a0..7e0d319 100644
	--- a/includes/parser/Parser.php
	+++ b/includes/parser/Parser.php
	@@ -115,7 +115,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>';
	@@ -346,7 +347,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 e965352..51c597a 100644
	--- a/tests/parser/parserTests.txt
	+++ b/tests/parser/parserTests.txt
	@@ -2125,6 +2125,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: 3678985
Default Alt Text: T110143b-REL1_25.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_25.patchcsteipp (Chris Steipp)Actions

T110143b-REL1_25.patchView Options

File Metadata

Event Timeline

T110143b-REL1_25.patch
csteipp (Chris Steipp)
Actions

T110143b-REL1_25.patch
View Options