From 1af2df5a650b517dabfdcb58c49583a71f4b2ef3 Mon Sep 17 00:00:00 2001
From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Tue, 22 Nov 2022 15:44:20 +0100
Subject: [PATCH] SECURITY: HTML-escape inner formatter in HtmlTimeFormatter
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

HtmlTimeFormatter assumed that the inner formatter returned HTML, but we
were actually using it with MwTimeIsoFormatter, which returns plain
text. Given that we weren’t actually using the HTML feature, let’s just
have HtmlTimeFormatter HTML-escape the inner formatter’s result.

Bug: T323592
Change-Id: I5051a6b96af65a981bab33f63e789fadb41a65a5
---
 lib/includes/Formatters/HtmlTimeFormatter.php         |  5 +++--
 .../phpunit/Formatters/HtmlTimeFormatterTest.php      | 11 +++++++++++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/lib/includes/Formatters/HtmlTimeFormatter.php b/lib/includes/Formatters/HtmlTimeFormatter.php
index ffbd7af538..7cbb39c162 100644
--- a/lib/includes/Formatters/HtmlTimeFormatter.php
+++ b/lib/includes/Formatters/HtmlTimeFormatter.php
@@ -41,7 +41,8 @@ class HtmlTimeFormatter implements ValueFormatter {
 	/**
 	 * @param FormatterOptions|null $options
 	 * @param ValueFormatter $dateTimeFormatter A value formatter that accepts TimeValue objects and
-	 *  returns the formatted date and time, but not the calendar model. Must return HTML.
+	 *  returns the formatted date and time, but not the calendar model.
+	 *  The formatter is assumed to return plain text (its output will be HTML-escaped).
 	 * @param ShowCalendarModelDecider $decider
 	 */
 	public function __construct(
@@ -70,7 +71,7 @@ public function format( $value ) {
 			throw new InvalidArgumentException( 'Data value type mismatch. Expected a TimeValue.' );
 		}
 
-		$formatted = $this->dateTimeFormatter->format( $value );
+		$formatted = htmlspecialchars( $this->dateTimeFormatter->format( $value ) );
 
 		if ( $this->decider->showCalendarModel( $value, $this->options ) ) {
 			$formatted .= '<sup class="wb-calendar-name">'
diff --git a/lib/tests/phpunit/Formatters/HtmlTimeFormatterTest.php b/lib/tests/phpunit/Formatters/HtmlTimeFormatterTest.php
index 6f44c26d2b..eba8e47e1c 100644
--- a/lib/tests/phpunit/Formatters/HtmlTimeFormatterTest.php
+++ b/lib/tests/phpunit/Formatters/HtmlTimeFormatterTest.php
@@ -119,6 +119,17 @@ public function timeFormatProvider() {
 		return $testCases;
 	}
 
+	public function testEscapesHtml(): void {
+		$dateTimeFormatter = $this->createMock( ValueFormatter::class );
+		$dateTimeFormatter->method( 'format' )
+			->willReturn( '<script>' );
+		$formatter = new HtmlTimeFormatter( null, $dateTimeFormatter );
+
+		$value = $this->getTimeValue( 'MOCKTIME', TimeValue::PRECISION_DAY, 'calendar' );
+		$this->assertSame( '&lt;script&gt;<sup class="wb-calendar-name">calendar</sup>',
+			$formatter->format( $value ) );
+	}
+
 	/**
 	 * @dataProvider invalidValueProvider
 	 */
-- 
2.37.2