Page MenuHomePhabricator

0001-SECURITY-Create-xss-language-code-feature.patch

Authored By
Lucas_Werkmeister_WMDE
Jul 12 2023, 4:46 PM
Size
10 KB
Referenced Files
None
Subscribers
None

0001-SECURITY-Create-xss-language-code-feature.patch

From 6ff813aa9cd264e780225e6dbf8eeb5938ea128c Mon Sep 17 00:00:00 2001
From: Lucas Werkmeister <lucas.werkmeister@wikimedia.de>
Date: Wed, 12 Jul 2023 17:44:30 +0200
Subject: [PATCH] SECURITY: Create 'xss' language code feature
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
This is not a security fix in itself, but it should only be made public
once the associated Phabricator task, as well as other tasks that were
found using this feature while in development, are all public.
This creates a new language code, 'xss', which is enabled using the
setting $wgUseXssLanguage (similar to how $wgUsePigLatinVariant enables
the 'en-x-piglatin' language code, and likewise defaults to false but
set to true in the development settings). In this language code, all
messages become “malicious”, trying to run some alert() JavaScript; if
any alert() actually fires in the browser, the message was not escaped
properly. ($wgRawHtmlMessages are exempt, since they’re already known
to be “unsafe” and require more rights to edit on-wiki.) Such a message
is generally a minor security issue; it effectively means that a user
with 'editinterface' right (such as a sysop, on many wikis) can run
arbitrary JS, without needing the 'editsitejs' right (normally
restricted to interface admins). Developers can use this language code
to easily check their code for escaping issue.
Bug: T340201
Change-Id: Ia9a1cf712b139fea5da72046e37035e6de39d8d5
---
docs/config-schema.yaml | 3 ++
docs/config-vars.php | 6 +++
includes/DevelopmentSettings.php | 2 +
includes/MainConfigNames.php | 6 +++
includes/MainConfigSchema.php | 16 ++++++++
includes/SetupDynamicConfig.php | 3 ++
includes/config-schema.php | 1 +
includes/language/LanguageNameUtils.php | 4 ++
includes/language/MessageCache.php | 18 +++++++++
.../includes/cache/MessageCacheTest.php | 40 +++++++++++++++++++
10 files changed, 99 insertions(+)
diff --git a/docs/config-schema.yaml b/docs/config-schema.yaml
index 1a7cd53f7f..10da7919ab 100755
--- a/docs/config-schema.yaml
+++ b/docs/config-schema.yaml
@@ -3014,6 +3014,9 @@ config-schema:
A link to /wiki/ would be redirected to /sr/Главна_страна
It is important that $wgArticlePath not overlap with possible values
of $wgVariantArticlePath.
+ UseXssLanguage:
+ default: false
+ description: 'Whether to enable the ''xss'' language code, used for development.'
LoginLanguageSelector:
default: false
description: |-
diff --git a/docs/config-vars.php b/docs/config-vars.php
index 4312ad62ce..8d61f1b15c 100755
--- a/docs/config-vars.php
+++ b/docs/config-vars.php
@@ -1860,6 +1860,12 @@
*/
$wgVariantArticlePath = null;
+/**
+ * Config variable stub for the UseXssLanguage setting, for use by phpdoc and IDEs.
+ * @see MediaWiki\MainConfigSchema::UseXssLanguage
+ */
+$wgUseXssLanguage = null;
+
/**
* Config variable stub for the LoginLanguageSelector setting, for use by phpdoc and IDEs.
* @see MediaWiki\MainConfigSchema::LoginLanguageSelector
diff --git a/includes/DevelopmentSettings.php b/includes/DevelopmentSettings.php
index 3ce9b9ca3b..af4653eea3 100644
--- a/includes/DevelopmentSettings.php
+++ b/includes/DevelopmentSettings.php
@@ -142,6 +142,8 @@
// Enable en-x-piglatin variant conversion for testing
$wgUsePigLatinVariant = true;
+// Enable xss language code for testing correct message escaping
+$wgUseXssLanguage = true;
// Enable the new wikitext mode for browser testing (T270240)
$wgVisualEditorEnableWikitext = true;
diff --git a/includes/MainConfigNames.php b/includes/MainConfigNames.php
index d1fed1d797..3b4417bed5 100755
--- a/includes/MainConfigNames.php
+++ b/includes/MainConfigNames.php
@@ -1875,6 +1875,12 @@ class MainConfigNames {
*/
public const VariantArticlePath = 'VariantArticlePath';
+ /**
+ * Name constant for the UseXssLanguage setting, for use with Config::get()
+ * @see MainConfigSchema::UseXssLanguage
+ */
+ public const UseXssLanguage = 'UseXssLanguage';
+
/**
* Name constant for the LoginLanguageSelector setting, for use with Config::get()
* @see MainConfigSchema::LoginLanguageSelector
diff --git a/includes/MainConfigSchema.php b/includes/MainConfigSchema.php
index 669edd233e..170a27fa72 100644
--- a/includes/MainConfigSchema.php
+++ b/includes/MainConfigSchema.php
@@ -4842,6 +4842,22 @@ public static function getDefaultDBerrorLogTZ( $localtimezone ) {
'default' => false,
];
+ /**
+ * Whether to enable the 'xss' language code, used for development.
+ * When enabled, the language code 'xss' (e.g. via ?uselang=xss)
+ * can be used to test correct message escaping at scale.
+ * In this "language", every message becomes an HTML snippet
+ * which attempts to alert the message key.
+ * Well-written code will correctly escape all of these messages.
+ * If any alerts are actually fired in the browser,
+ * the message is not being escaped correctly;
+ * either the offending code should be fixed,
+ * or the message should be added to {@link self::RawHtmlMessages}.
+ */
+ public const UseXssLanguage = [
+ 'default' => false,
+ ];
+
/**
* Show a bar of language selection links in the user login and user
* registration forms; edit the "loginlanguagelinks" message to
diff --git a/includes/SetupDynamicConfig.php b/includes/SetupDynamicConfig.php
index 7b2bbc86de..3359bc579a 100644
--- a/includes/SetupDynamicConfig.php
+++ b/includes/SetupDynamicConfig.php
@@ -269,6 +269,9 @@
}
unset( $code ); // no global pollution; destroy reference
unset( $bcp47 ); // no global pollution; destroy reference
+if ( $wgUseXssLanguage ) {
+ $wgDummyLanguageCodes['xss'] = 'xss'; // Used for testing
+}
// Temporary backwards-compatibility reading of old replica lag settings as of MediaWiki 1.36,
// to support sysadmins who fail to update their settings immediately:
diff --git a/includes/config-schema.php b/includes/config-schema.php
index e9838563b6..455319f201 100755
--- a/includes/config-schema.php
+++ b/includes/config-schema.php
@@ -611,6 +611,7 @@
'DisabledVariants' => [
],
'VariantArticlePath' => false,
+ 'UseXssLanguage' => false,
'LoginLanguageSelector' => false,
'ForceUIMsgAsContentMsg' => [
],
diff --git a/includes/language/LanguageNameUtils.php b/includes/language/LanguageNameUtils.php
index c6c7ff8df9..c7480fa8ee 100644
--- a/includes/language/LanguageNameUtils.php
+++ b/includes/language/LanguageNameUtils.php
@@ -80,6 +80,7 @@ class LanguageNameUtils {
public const CONSTRUCTOR_OPTIONS = [
MainConfigNames::ExtraLanguageNames,
MainConfigNames::UsePigLatinVariant,
+ MainConfigNames::UseXssLanguage,
];
/** @var HookRunner */
@@ -234,6 +235,9 @@ private function getLanguageNamesUncached( $inLanguage, $include ) {
// Suppress Pig Latin unless explicitly enabled.
unset( $mwNames['en-x-piglatin'] );
}
+ if ( $this->options->get( MainConfigNames::UseXssLanguage ) ) {
+ $mwNames['xss'] = 'fake xss language (see $wgUseXssLanguage)';
+ }
foreach ( $mwNames as $mwCode => $mwName ) {
# - Prefer own MediaWiki native name when not using the hook
diff --git a/includes/language/MessageCache.php b/includes/language/MessageCache.php
index 7ac00ef8e3..a1a91d4727 100644
--- a/includes/language/MessageCache.php
+++ b/includes/language/MessageCache.php
@@ -61,6 +61,8 @@ class MessageCache implements LoggerAwareInterface {
MainConfigNames::UseDatabaseMessages,
MainConfigNames::MaxMsgCacheEntrySize,
MainConfigNames::AdaptiveMessageCache,
+ MainConfigNames::UseXssLanguage,
+ MainConfigNames::RawHtmlMessages,
];
/**
@@ -116,6 +118,12 @@ class MessageCache implements LoggerAwareInterface {
/** @var bool */
private $adaptive;
+ /** @var bool */
+ private $useXssLanguage;
+
+ /** @var string[] */
+ private $rawHtmlMessages;
+
/**
* Message cache has its own parser which it uses to transform messages
* @var ParserOptions
@@ -226,6 +234,8 @@ public function __construct(
$this->disable = !$options->get( MainConfigNames::UseDatabaseMessages );
$this->maxEntrySize = $options->get( MainConfigNames::MaxMsgCacheEntrySize );
$this->adaptive = $options->get( MainConfigNames::AdaptiveMessageCache );
+ $this->useXssLanguage = $options->get( MainConfigNames::UseXssLanguage );
+ $this->rawHtmlMessages = $options->get( MainConfigNames::RawHtmlMessages );
}
public function setLogger( LoggerInterface $logger ) {
@@ -1203,6 +1213,14 @@ private function getMessageForLang( $lang, $lckey, $useDB, &$alreadyTried ) {
// TODO: Move to a better place.
if ( $langcode === 'qqx' ) {
return '($*)';
+ } elseif (
+ $langcode === 'xss' &&
+ $this->useXssLanguage &&
+ !( in_array( $lckey, $this->rawHtmlMessages, true ) )
+ ) {
+ $xssViaInnerHtml = "<script>alert('$lckey')</script>";
+ $xssViaAttribute = '">' . $xssViaInnerHtml . '<x y="';
+ return $xssViaInnerHtml . $xssViaAttribute;
}
// Check the localisation cache
diff --git a/tests/phpunit/includes/cache/MessageCacheTest.php b/tests/phpunit/includes/cache/MessageCacheTest.php
index ba1b332004..756845b251 100644
--- a/tests/phpunit/includes/cache/MessageCacheTest.php
+++ b/tests/phpunit/includes/cache/MessageCacheTest.php
@@ -316,4 +316,44 @@ public static function provideLocalOverride() {
[ 'nstab-help' ],
];
}
+
+ /** @dataProvider provideXssLanguage */
+ public function testXssLanguage( array $config, bool $expectXssMessage ): void {
+ $this->overrideConfigValues( $config + [
+ MainConfigNames::UseXssLanguage => false,
+ MainConfigNames::RawHtmlMessages => [],
+ ] );
+
+ $message = $this->getServiceContainer()->getMessageCache()->get( 'key', true, 'xss' );
+ if ( $expectXssMessage ) {
+ $this->assertSame(
+ "<script>alert('key')</script>\"><script>alert('key')</script><x y=\"",
+ $message
+ );
+ } else {
+ $this->assertFalse( $message );
+ }
+ }
+
+ public static function provideXssLanguage(): iterable {
+ yield 'default' => [
+ 'config' => [],
+ 'expectXssMessage' => false,
+ ];
+
+ yield 'enabled' => [
+ 'config' => [
+ MainConfigNames::UseXssLanguage => true,
+ ],
+ 'expectXssMessage' => true,
+ ];
+
+ yield 'enabled but message marked as raw' => [
+ 'config' => [
+ MainConfigNames::UseXssLanguage => true,
+ MainConfigNames::RawHtmlMessages => [ 'key' ],
+ ],
+ 'expectXssMessage' => false,
+ ];
+ }
}
--
2.39.2

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
10920870
Default Alt Text
0001-SECURITY-Create-xss-language-code-feature.patch (10 KB)

Event Timeline