From fd3fad2ed872d9ce62d6c23d28ff5bd94b3d18a1 Mon Sep 17 00:00:00 2001 From: "C. Scott Ananian" Date: Mon, 4 Aug 2025 16:43:00 +0200 Subject: [PATCH] SECURITY: Sanitize data- attributes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, if you managed to get data- attributes with e.g spaces or slashes in the name into validateAttributes(), then the rest of the attribute name would not be validated and get concatenated into HTML that would eventually be parsed as separate attributes (or even tag contents and new markup, if you had a > in the name). I don’t think this was possible via regular

parsing, as decodeTagAttributes() would decode the attributes differently in that case, but it was possible via various wikitext constructs, including {{#tag:}}. Tighten the regex to throw out such invalid attributes, and add a few tests in this direction. More refactoring, and especially more tests, can happen later, once this chaneg is public and we can benefit from CI. Bug: T401099 Change-Id: SECURITY-Id095a3278083dbedba083d5aa3c1cbaa379a682f Co-Authored-By: Lucas Werkmeister --- includes/parser/Sanitizer.php | 5 ++++- .../phpunit/includes/parser/SanitizerTest.php | 19 +++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/includes/parser/Sanitizer.php b/includes/parser/Sanitizer.php index 2d6934bb93..077bae29f8 100644 --- a/includes/parser/Sanitizer.php +++ b/includes/parser/Sanitizer.php @@ -512,8 +512,11 @@ public static function validateAttributes( array $attribs, array $allowed ): arr # * Disallow data attributes used by MediaWiki code # * Ensure that the attribute is not namespaced by banning # colons. + # * Ensure attribute name will be accepted by the HTML + # parser; see + # https://github.com/whatwg/dom/issues/849#issuecomment-1007541209 if ( ( - !preg_match( '/^data-[^:]*$/i', $attribute ) && + !preg_match( '|^data-[^:= \t\r\n/>\0]*$|i', $attribute ) && !array_key_exists( $attribute, $allowed ) ) || self::isReservedDataAttribute( $attribute ) ) { continue; diff --git a/tests/phpunit/includes/parser/SanitizerTest.php b/tests/phpunit/includes/parser/SanitizerTest.php index 24b5acf140..68f85fc471 100644 --- a/tests/phpunit/includes/parser/SanitizerTest.php +++ b/tests/phpunit/includes/parser/SanitizerTest.php @@ -160,6 +160,25 @@ public static function provideValidateTagAttributes() { [ 'role' => 'menuitem', 'aria-hidden' => 'false' ], [ 'role' => 'menuitem', 'aria-hidden' => 'false' ], ], + [ 'div', + [ + 'data-wikitext' => 'wikitext', + 'DATA-WIKITEXT-2' => 'WIKITEXT-2', + 'data-mw' => 'disallow impersonating parsoid', + 'DATA-mw' => 'disallow impersonating PARSOID', + 'data-mw-extension' => 'disallow impersonating extension', + 'data-:namespaced' => 'disallow namespace', + 'data- invalid' => 'disallow XSS', + 'data-/invalid' => 'disallow XSS', + 'data->invalid' => 'disallow XSS', + 'data-=invalid' => 'disallow XSS', + ], + [ + 'data-wikitext' => 'wikitext', + 'DATA-WIKITEXT-2' => 'WIKITEXT-2', + # other attributes removed + ] + ], ]; } -- 2.50.1