SECURITY: Reject stylesheets containing "</style"

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

Bug: T167812
Change-Id: I34c5f200c689a56d340bce70ffebbf58d27b499e

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" => '<27>' ] );

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>&lt;/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.",

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>&lt;/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.",

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,