0001-SECURITY-Reject-stylesheets-containing-style.patch
Anomie
Actions

Authored By

	Anomie
	Jun 13 2017, 3:56 PM

Size

5 KB

Referenced Files

None

Subscribers

None

0001-SECURITY-Reject-stylesheets-containing-style.patch
View Options

	From b04bd96f58135bcf2d1ebfd791fdadba073df4f1 Mon Sep 17 00:00:00 2001
	From: Brad Jorsch <bjorsch@wikimedia.org>
	Date: Tue, 13 Jun 2017 11:52:07 -0400
	Subject: [PATCH 1/2] SECURITY: Reject stylesheets containing "</style"

	Premature closing of the style block === HTML injection vector.

	Bug: T167812
	Change-Id: I34c5f200c689a56d340bce70ffebbf58d27b499e
	---
	TemplateStylesContent.php \| 10 +++++++++-
	i18n/en.json \| 1 +
	i18n/qqq.json \| 1 +
	tests/phpunit/TemplateStylesContentTest.php \| 28 ++++++++++++++++++++++++++++
	4 files changed, 39 insertions(+), 1 deletion(-)

	diff --git a/TemplateStylesContent.php b/TemplateStylesContent.php
	index c56b92c..05983be 100644
	--- a/TemplateStylesContent.php
	+++ b/TemplateStylesContent.php
	@@ -83,8 +83,16 @@ class TemplateStylesContent extends TextContent {
	$sanitizer->clearSanitizationErrors();

	// Stringify it while minifying
	+ $value = CSSUtil::stringify( $stylesheet, [ 'minify' => $options['minify'] ] );
	+
	+ // Sanity check, don't allow "</style" if one somehow sneaks through the sanitizer
	+ if ( preg_match( '!</style!i', $value ) ) {
	+ $value = '';
	+ $status->fatal( 'templatestyles-end-tag-injection' );
	+ }
	+
	if ( !$options['novalue'] ) {
	- $status->value = CSSUtil::stringify( $stylesheet, [ 'minify' => $options['minify'] ] );
	+ $status->value = $value;

	// Sanity check, don't allow raw U+007F if one somehow sneaks through the sanitizer
	$status->value = strtr( $status->value, [ "\x7f" => '�' ] );
	diff --git a/i18n/en.json b/i18n/en.json
	index 7b56e5d..d688d06 100644
	--- a/i18n/en.json
	+++ b/i18n/en.json
	@@ -13,6 +13,7 @@
	"templatestyles-bad-src": "Page [[:$1\|$2]] must have content model \"{{int:content-model-sanitized-css}}\" for TemplateStyles (current model is \"$3\").",
	"templatestyles-errorcomment": "Errors processing stylesheet [[:$1]] (rev $2):\n$3",
	"templatestyles-size-exceeded": "The stylesheet is larger than the maximum size of $2.",
	+ "templatestyles-end-tag-injection": "The supplied stylesheet contains <code></style</code>, which is not allowed.",
	"content-model-sanitized-css": "Sanitized CSS",

	"templatestyles-error-at-rule-block-not-allowed": "Block not allowed for <code>@$3</code> at line $1 character $2.",
	diff --git a/i18n/qqq.json b/i18n/qqq.json
	index a1157e7..44b5629 100644
	--- a/i18n/qqq.json
	+++ b/i18n/qqq.json
	@@ -13,6 +13,7 @@
	"templatestyles-bad-src": "Error message displayed when the title specified is not a usable stylesheet. Parameters:\n* $1 - The title specified.\n* $2 - The title with wikitext escaped.\n* $3 - Current content model of the page in question.",
	"templatestyles-errorcomment": "Formatting for the comment used to display TemplateStyles errors encountered during the parse. Parameters:\n* $1 - Source stylesheet.\n* $2 - Revision of the stylesheet.* $3 - Errors.",
	"templatestyles-size-exceeded": "Error returned when the stylesheet is more than $wgTemplateStylesMaxStylesheetSize bytes. Parameters:\n* $1 - Maximum size in bytes\n* $2 - Maximum size in \"human units\" (i.e. KB, MB, GB, etc).",
	+ "templatestyles-end-tag-injection": "Error returned when the stylesheet contains <code></style</code>, which is likely an attempt at HTML injection.",
	"content-model-sanitized-css": "Name for TemplateStyles sanitized-css content model.",
	"templatestyles-error-at-rule-block-not-allowed": "Error in CSS validation. Note \"block\" in this message refers to the part of CSS syntax inside the <code>{ ... }</code>. Parameters:\n* $1 - Line number of the error.\n* $2 - Location of the error within the line.\n* $3 - Name of the at-rule in question.",
	"templatestyles-error-at-rule-block-required": "Error in CSS validation. Note \"block\" in this message refers to the part of CSS syntax inside the <code>{ ... }</code>. Parameters:\n* $1 - Line number of the error.\n* $2 - Location of the error within the line.\n* $3 - Name of the at-rule in question.",
	diff --git a/tests/phpunit/TemplateStylesContentTest.php b/tests/phpunit/TemplateStylesContentTest.php
	index 43d22af..0b32538 100644
	--- a/tests/phpunit/TemplateStylesContentTest.php
	+++ b/tests/phpunit/TemplateStylesContentTest.php
	@@ -80,6 +80,34 @@ class TemplateStylesContentTest extends TextContentTest {
	];
	}

	+ /**
	+ * @dataProvider provideHtmlInjection
	+ * @param string $text Input text
	+ * @param string $output Valid escaped output text
	+ */
	+ public function testHtmlInjection( $text, $output ) {
	+ $status = $this->newContent( $text )->sanitize();
	+
	+ if ( $status->isOk() ) { // css-sanitizer 1.0.2+
	+ $this->assertEquals( Status::newGood( $output ), $status );
	+ } else { // css-sanitizer 1.0.1
	+ $this->assertTrue( $status->hasMessage( 'templatestyles-end-tag-injection' ) );
	+ }
	+ }
	+
	+ public static function provideHtmlInjection() {
	+ return [
	+ '</style> in string' => [
	+ '.foo { content: "</style>"; }',
	+ '.mw-parser-output .foo{content:"\3c /style\3e "}',
	+ ],
	+ '</style> via identifiers' => [
	+ '.foo { grid-area: \< / style 0 / \>; }',
	+ '.mw-parser-output .foo{grid-area:\3c /style 0/\3e }',
	+ ],
	+ ];
	+ }
	+
	public function testSizeLimit() {
	$this->setMwGlobals( [
	'wgTemplateStylesMaxStylesheetSize' => 10,
	--
	2.11.0

File Metadata

Mime Type: text/x-diff
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 4713825
Default Alt Text: 0001-SECURITY-Reject-stylesheets-containing-style.patch (5 KB)

0001-SECURITY-Reject-stylesheets-containing-style.patchAnomieActions

0001-SECURITY-Reject-stylesheets-containing-style.patchView Options

File Metadata

Event Timeline

0001-SECURITY-Reject-stylesheets-containing-style.patch
Anomie
Actions

0001-SECURITY-Reject-stylesheets-containing-style.patch
View Options