Page MenuHomePhabricator
Files F37853187

T347708-2.patch
Dreamy_Jazz (WBrown (WMF))
Actions

Authored By
Dreamy_Jazz
Sep 29 2023, 5:14 PM
Size
6 KB
Referenced Files
None
Subscribers
None

T347708-2.patch
View Options

From eec5cb196e0f0036bcd4ee880ebff4f2aeb1ede3 Mon Sep 17 00:00:00 2001
From: Dreamy Jazz <wbrown-ctr@wikimedia.org>
Date: Fri, 29 Sep 2023 15:57:12 +0100
Subject: [PATCH] SECURITY: Address many XSS vectors via message definitions
Why:
* Several messages in the CheckUser extension allow users without
editsitecss or editsitejs to add CSS and/or JS code that is
viewable by users using the CheckUser interfaces.
* These interfaces should not allow CSS and/or JS code to be
injected in this way and should properly escape HTML unless
the use of HTML code is required.
What:
* Define a few SpecialCheckUserLog result line messages as raw
HTML messages by adding them to wgRawHtmlMessages as they
contain bidirectional isolation HTML elements.
* Update the code in Special:CheckUserLog to pass the parameters
as raw parameters and also use ::parse to remove dangerous
HTML but allow the use of bdi elements.
* Update SpecialCheckUserLog to always escape the 'parentheses'
message.
* Update CheckUserGetUsersPager to escape the
'checkuser-massblock-text' message.
* Change ApiQueryCheckUser to escape the 'checkuser-reason-api'
message.
* Update SpecialInvestigate to escape the Language::userDate
output.
Bug: T347708
Change-Id: If3ce02cac9c5f2a6f84c42d902b8290eb1fa7250
---
src/Api/ApiQueryCheckUser.php | 2 +-
.../Pagers/CheckUserGetUsersPager.php | 2 +-
src/CheckUser/Pagers/CheckUserLogPager.php | 43 ++++++++++---------
src/Investigate/Pagers/TimelinePager.php | 4 +-
4 files changed, 26 insertions(+), 25 deletions(-)
diff --git a/src/Api/ApiQueryCheckUser.php b/src/Api/ApiQueryCheckUser.php
index 7d578177..86f4c627 100644
--- a/src/Api/ApiQueryCheckUser.php
+++ b/src/Api/ApiQueryCheckUser.php
@@ -74,7 +74,7 @@ class ApiQueryCheckUser extends ApiQueryBase {
$this->dieWithError( 'apierror-checkuser-missingsummary', 'missingdata' );
}
- $reason = $this->msg( 'checkuser-reason-api', $reason )->inContentLanguage()->text();
+ $reason = $this->msg( 'checkuser-reason-api', $reason )->inContentLanguage()->escaped();
// absolute time
$timeCutoff = strtotime( $timecond );
if ( !$timeCutoff || $timeCutoff < 0 || $timeCutoff > time() ) {
diff --git a/src/CheckUser/Pagers/CheckUserGetUsersPager.php b/src/CheckUser/Pagers/CheckUserGetUsersPager.php
index 01077c50..3adfa3b3 100644
--- a/src/CheckUser/Pagers/CheckUserGetUsersPager.php
+++ b/src/CheckUser/Pagers/CheckUserGetUsersPager.php
@@ -654,7 +654,7 @@ class CheckUserGetUsersPager extends AbstractCheckUserPager {
->setSubmitTextMsg( 'checkuser-massblock-commit' )
->setSubmitId( 'checkuserblocksubmit' )
->setSubmitName( 'checkuserblock' )
- ->setHeaderHtml( $this->msg( 'checkuser-massblock-text' )->text() );
+ ->setHeaderHtml( $this->msg( 'checkuser-massblock-text' )->escaped() );
if ( $config->get( 'BlockAllowsUTEdit' ) ) {
$fieldset->addFields( [
diff --git a/src/CheckUser/Pagers/CheckUserLogPager.php b/src/CheckUser/Pagers/CheckUserLogPager.php
index c08ca294..fe90ee04 100644
--- a/src/CheckUser/Pagers/CheckUserLogPager.php
+++ b/src/CheckUser/Pagers/CheckUserLogPager.php
@@ -151,14 +151,14 @@ class CheckUserLogPager extends RangeChronologicalPager {
}
$user .= $this->msg( 'word-separator' )->escaped()
. Html::rawElement( 'span', [ 'classes' => 'mw-usertoollinks' ],
- $this->msg( 'parentheses' )->params( $this->getLinkRenderer()->makeLink(
+ $this->msg( 'parentheses' )->rawParams( $this->getLinkRenderer()->makeLink(
SpecialPage::getTitleFor( 'CheckUserLog' ),
$this->msg( 'checkuser-log-checks-by' )->text(),
[],
[
'cuInitiator' => $row->actor_name,
]
- ) )->text()
+ ) )->escaped()
);
}
@@ -192,26 +192,27 @@ class CheckUserLogPager extends RangeChronologicalPager {
// Give grep a chance to find the usages:
// checkuser-log-entry-userips, checkuser-log-entry-ipedits,
// checkuser-log-entry-ipusers, checkuser-log-entry-ipedits-xff
- // checkuser-log-entry-ipusers-xff, checkuser-log-entry-useredits
- $rowContent = $this->msg(
- 'checkuser-log-entry-' . $row->cul_type,
- $user,
- $target,
- $this->generateTimestampLink(
- $lang->userTimeAndDate(
- wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser
+ // checkuser-log-entry-ipusers-xff, checkuser-log-entry-useredits,
+ // checkuser-log-entry-investigate
+ $rowContent = $this->msg( 'checkuser-log-entry-' . $row->cul_type )
+ ->rawParams(
+ $user,
+ $target,
+ $this->generateTimestampLink(
+ $lang->userTimeAndDate(
+ wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser
+ ),
+ $row
),
- $row
- ),
- $this->generateTimestampLink(
- $lang->userDate( wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser ),
- $row
- ),
- $this->generateTimestampLink(
- $lang->userTime( wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser ),
- $row
- )
- )->text();
+ $this->generateTimestampLink(
+ $lang->userDate( wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser ),
+ $row
+ ),
+ $this->generateTimestampLink(
+ $lang->userTime( wfTimestamp( TS_MW, $row->cul_timestamp ), $contextUser ),
+ $row
+ )
+ )->parse();
$rowContent .= $this->commentFormatter->formatBlock(
$this->commentStore->getComment( 'cul_reason', $row )->text
);
diff --git a/src/Investigate/Pagers/TimelinePager.php b/src/Investigate/Pagers/TimelinePager.php
index 345de6c2..aa05b148 100644
--- a/src/Investigate/Pagers/TimelinePager.php
+++ b/src/Investigate/Pagers/TimelinePager.php
@@ -109,14 +109,14 @@ class TimelinePager extends ReverseChronologicalPager {
$dateHeader = $this->getLanguage()->userDate( wfTimestamp( TS_MW, $row->cuc_timestamp ), $this->getUser() );
if ( $this->lastDateHeader === null ) {
$this->lastDateHeader = $dateHeader;
- $line .= Html::rawElement( 'h4', [], $dateHeader );
+ $line .= Html::element( 'h4', [], $dateHeader );
$line .= Html::openElement( 'ul' );
} elseif ( $this->lastDateHeader !== $dateHeader ) {
$this->lastDateHeader = $dateHeader;
// Start a new list with a new date header
$line .= Html::closeElement( 'ul' );
- $line .= Html::rawElement( 'h4', [], $dateHeader );
+ $line .= Html::element( 'h4', [], $dateHeader );
$line .= Html::openElement( 'ul' );
}
--
2.25.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
11512930
Default Alt Text
T347708-2.patch (6 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits