Page MenuHomePhabricator

01-T323592-rev2.patch

Authored By
sbassett
Nov 30 2022, 12:38 AM
Size
2 KB
Referenced Files
None
Subscribers
None

01-T323592-rev2.patch

From ef0c1cddb223ae4e750814180de8962caf239e9a Mon Sep 17 00:00:00 2001
From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Tue, 29 Nov 2022 18:34:20 -0600
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
---
lib/includes/Formatters/HtmlTimeFormatter.php | 6 +++---
.../phpunit/Formatters/HtmlTimeFormatterTest.php | 11 +++++++++++
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/lib/includes/Formatters/HtmlTimeFormatter.php b/lib/includes/Formatters/HtmlTimeFormatter.php
index ffbd7af538..90d0a2a66c 100644
--- a/lib/includes/Formatters/HtmlTimeFormatter.php
+++ b/lib/includes/Formatters/HtmlTimeFormatter.php
@@ -41,8 +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.
- * @param ShowCalendarModelDecider $decider
+ * 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).
*/
public function __construct(
?FormatterOptions $options,
@@ -70,7 +70,7 @@ class HtmlTimeFormatter implements ValueFormatter {
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 @@ class HtmlTimeFormatterTest extends \PHPUnit\Framework\TestCase {
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.0 (Apple Git-136)

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
9919570
Default Alt Text
01-T323592-rev2.patch (2 KB)

Event Timeline