Page MenuHomePhabricator
Create Task
Maniphest T357760

CVE-2024-: Denial of service vector via GET request to Special:MovePage on pages with thousands of subpages
Closed, ResolvedPublicSecurity
Actions

Assigned To
Dreamy_Jazz
Authored By
Dreamy_Jazz
Feb 16 2024, 10:22 AM
Tags
Referenced Files
F43444543: T357760-v2-REL1_39.patch
Mar 26 2024, 3:24 PM
F43444539: T357760-v2-REL1_40.patch
Mar 26 2024, 3:24 PM
F43444161: T357760-v2.patch
Mar 26 2024, 3:15 PM
F41970839: T357760.patch
Feb 18 2024, 11:28 PM
Unknown Object (File)
Feb 18 2024, 11:27 PM
Unknown Object (File)
Feb 18 2024, 10:56 PM
F41930037: image.png
Feb 16 2024, 3:00 PM
Subscribers
Aklapper
DannyS712
Dreamy_Jazz
gerritbot
mmartorana
Mstyles
Reedy
sbassett

Description

If a user with the necessary rights to move the page opens Special:MovePage for a page with tens of thousands of subpages, then the page will exceed the maximum request time.

Steps to re-produce
  1. Log into an account with the move right
  2. Find a page with around tens of thousands of subpages which is unprotected or you have the rights to move
  3. Open Special:MovePage for this page

What happened: The request times out
What should have happened: The page should refuse to display the subpages above a certain threshold to avoid having the request timeout and/or prevent moving the page.

Extra details

The example that I found which prompted me to file this ticket is opening up the move page for https://en.wikipedia.org/wiki/Wikipedia:Sockpuppet_investigations. I had accidentally opened the page at first and wondered why the request took so long (the first request did not timeout but took ages to run). I opened the move page while on mwdebug2001 and then while I was not on a debug server to get the relevant logstash logs. Both of these requests timed out:

Based on the logs from the mwdebug2001 request it looks like the cause of the timeout is LinkCache as all logstash logs are to do with this class from 09:46:33.961 to 09:47:31.645 (when the timeout occurs). The logs that appear in the debug request say that 960 DB lookups occur because the LinkCache does not have the page cached. The last log before the timeout is this one which suggests that the LinkCache was nowhere near the end of the list of pages to process.

While the example I found has move protection such that only administrators can load the page, it does not make it unlikely that there are other pages with enough subpages to cause a timeout but do not have move protection. Furthermore, as this is done through a GET request to open a form there is no on-wiki logs that would flag that someone abusing this (instead monitoring logstash would find this).

Details

Risk Rating
Low
Author Affiliation
WMF Technology Dept
SubjectRepoBranchLines +/-
Use i18n strings for truncated subpage message in SpecialMovePagemediawiki/coreREL1_39+15 -12
Use i18n strings for truncated subpage message in SpecialMovePagemediawiki/coremaster+94 -12
Use i18n strings for truncated subpage message in SpecialMovePagemediawiki/coreREL1_40+94 -12
Use i18n strings for truncated subpage message in SpecialMovePagemediawiki/coreREL1_41+94 -12
SECURITY: Limit subpages displayed on Special:MovePage formmediawiki/coremaster+27 -3
SECURITY: Limit subpages displayed on Special:MovePage formmediawiki/coreREL1_40+27 -3
SECURITY: Limit subpages displayed on Special:MovePage formmediawiki/coreREL1_41+27 -3
SECURITY: Limit subpages displayed on Special:MovePage formmediawiki/coreREL1_39+27 -3
Customize query in gerrit

Related Objects
Search...

Event Timeline

Dreamy_Jazz created this task.Feb 16 2024, 10:22 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 16 2024, 10:22 AM
Dreamy_Jazz renamed this task from Denial of service vector via Special:Move to Denial of service vector via Special:Move on pages with thousands of subpages.Feb 16 2024, 10:22 AM
Dreamy_Jazz renamed this task from Denial of service vector via Special:Move on pages with thousands of subpages to Denial of service vector via GET request to Special:Move on pages with thousands of subpages.
Dreamy_Jazz updated the task description. (Show Details)
Dreamy_Jazz added projects: Vuln-DoS, MediaWiki-Special-pages.Feb 16 2024, 10:25 AM
Dreamy_Jazz edited projects, added MediaWiki-Page-rename; removed MediaWiki-Special-pages.
Dreamy_Jazz updated the task description. (Show Details)Feb 16 2024, 10:29 AM
Dreamy_Jazz updated the task description. (Show Details)
DannyS712 subscribed.Feb 16 2024, 2:54 PM

Doesn't this require the rights to move subpages though, i.e. generally only exploitable by advanced users?

Dreamy_Jazz added a comment.EditedFeb 16 2024, 3:00 PM
In T357760#9550746, @DannyS712 wrote:

Doesn't this require the rights to move subpages though, i.e. generally only exploitable by advanced users?

No, because that right is only checked when the user submits the form. For example, if I grant the move right to all users (using *) I can still load the form while logged out for a page which has subpages:

image.png (786×1 px, 47 KB)

The DoS vector isn't moving the pages and instead is the thousands of queries used to create the list of subpages shown underneath the Subpages section.

Dreamy_Jazz updated the task description. (Show Details)Feb 16 2024, 3:00 PM
Dreamy_Jazz updated the task description. (Show Details)Feb 16 2024, 3:05 PM
Dreamy_Jazz added a comment.Feb 16 2024, 3:20 PM

A quick fix for this is to provide a $limit to Title::getSubpages in SpecialMovePage::showSubpages as the code currently provides no limit. Limiting this to the maximum number of subpages that can be moved should be fine, as if there are any more subpages this would mean that the form would not allow moving subpages anyway.

If a user wants to find the full list they could use Special:PrefixIndex which allows paging.

Dreamy_Jazz renamed this task from Denial of service vector via GET request to Special:Move on pages with thousands of subpages to Denial of service vector via GET request to Special:MovePage on pages with thousands of subpages.Feb 18 2024, 8:48 PM
Dreamy_Jazz updated the task description. (Show Details)
Dreamy_Jazz updated the task description. (Show Details)Feb 18 2024, 10:31 PM
Dreamy_Jazz updated the task description. (Show Details)
Dreamy_Jazz added a project: Patch-For-Review.Feb 18 2024, 10:56 PM
This comment was removed by Dreamy_Jazz.
Dreamy_Jazz added a comment.EditedFeb 18 2024, 11:27 PM

Proposed patch:

T357760.patch4 KBDownload

Note: Due to https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Guidelines_for_creating_patches, this patch has a hardcoded message which is used when the list of subpages is truncated.

Dreamy_Jazz attached a referenced file: F41970839: T357760.patch. (Show Details)Feb 18 2024, 11:28 PM
mmartorana changed the task status from Open to In Progress.Feb 26 2024, 1:14 PM
mmartorana triaged this task as Low priority.
mmartorana changed Risk Rating from N/A to Low.
mmartorana moved this task from Incoming to Security Patch To Deploy on the Security-Team board.Feb 28 2024, 5:51 PM
mmartorana subscribed.

From a security perspective, there doesn't seem to be any concern with this patch.

sbassett subscribed.Feb 28 2024, 6:15 PM
In T357760#9584799, @mmartorana wrote:

From a security perspective, there doesn't seem to be any concern with this patch.

Ok, I suppose we could try to get this deployed by next Monday's security deployment window (2024-03-04) at the latest.

Mstyles subscribed.Mar 4 2024, 10:47 PM
In T357760#9553901, @Dreamy_Jazz wrote:

Proposed patch:

T357760.patch4 KBDownload

Note: Due to https://wikitech.wikimedia.org/wiki/How_to_deploy_code#Guidelines_for_creating_patches, this patch has a hardcoded message which is used when the list of subpages is truncated.

Deployed

Mstyles moved this task from Security Patch To Deploy to Watching on the Security-Team board.Mar 4 2024, 10:48 PM
Mstyles added a parent task: T353895: Tracking bug for MediaWiki 1.39.7/1.40.3/1.41.1.Mar 4 2024, 10:51 PM
Dreamy_Jazz added a comment.Mar 4 2024, 11:08 PM
In T357760#9598683, @Mstyles wrote:

Deployed

Thanks. I've been able to verify that this patch works as expected and prevents the request timeout.

There is one bug I didn't find when testing. The hard-coded message is switched around (so the message for the talk pages is above the non-talk pages and vice. versa.). That isn't a major problem IMO and could be fixed once this is uploaded to gerrit.

Mstyles added a comment.Mar 5 2024, 6:50 AM

@Dreamy_Jazz I'm glad that the patch works. I think adding the hard-coded message once this uploaded is fine. There was an issue with deployment which I wanted to note here: https://phabricator.wikimedia.org/T276237#9598800

sbassett added a comment.Mar 5 2024, 3:44 PM
In T357760#9599310, @Mstyles wrote:

@Dreamy_Jazz I'm glad that the patch works. I think adding the hard-coded message once this uploaded is fine. There was an issue with deployment which I wanted to note here: https://phabricator.wikimedia.org/T276237#9598800

This week's train will probably just "fix" this issue, but we should keep an eye on T359114 to see when it gets resolved.

Reedy subscribed.Mar 26 2024, 2:52 PM
In T357760#9598730, @Dreamy_Jazz wrote:
In T357760#9598683, @Mstyles wrote:

Deployed

Thanks. I've been able to verify that this patch works as expected and prevents the request timeout.

There is one bug I didn't find when testing. The hard-coded message is switched around (so the message for the talk pages is above the non-talk pages and vice. versa.). That isn't a major problem IMO and could be fixed once this is uploaded to gerrit.

Noting that when patches are uploaded to gerrit, that is after they've been released in tarballs.

Reedy renamed this task from Denial of service vector via GET request to Special:MovePage on pages with thousands of subpages to CVE-2024-: Denial of service vector via GET request to Special:MovePage on pages with thousands of subpages.Mar 26 2024, 2:53 PM
Reedy added a subscriber: gerritbot.Mar 26 2024, 3:12 PM
Reedy added a comment.Mar 26 2024, 3:15 PM
diff --git a/includes/specials/SpecialMovePage.php b/includes/specials/SpecialMovePage.php
index d35999b190c..eed3e74a1d7 100644
--- a/includes/specials/SpecialMovePage.php
+++ b/includes/specials/SpecialMovePage.php
@@ -1030,7 +1030,7 @@ class SpecialMovePage extends UnlistedSpecialPage {
                        //  having the i18n rebuilt for all deployments due to this security patch.
                        $out->addWikiTextAsInterface(
                                "The first $maximumMovedPages {{PLURAL:$maximumMovedPages|subpage|subpages}} " .
-                               ( $noSubpageMsg ? 'for the corresponding talk page' : 'for this page' ) . ' are shown below.'
+                               ( $noSubpageMsg ? 'for this page' : 'for the corresponding talk page' ) . ' are shown below.'
                        );
                } else {
                        $out->addWikiMsg( $wikiMsg, $this->getLanguage()->formatNum( $pagecount ) );

T357760-v2.patch4 KBDownload

Reedy added a comment.Mar 26 2024, 3:24 PM

Patch applies cleanly to MW-1.41-release and master.

rMWa99ec1b4fa59: Title: Use TitleArrayFromResult instead of TitleArray seems to cause the conflicts on MW-1.40-release

T357760-v2-REL1_40.patch4 KBDownload

And applying that patch with -3 works for MW-1.39-release (file rename)

T357760-v2-REL1_39.patch4 KBDownload

Dreamy_Jazz added a comment.Mar 27 2024, 5:44 PM
In T357760#9661953, @Reedy wrote:
diff --git a/includes/specials/SpecialMovePage.php b/includes/specials/SpecialMovePage.php
index d35999b190c..eed3e74a1d7 100644
--- a/includes/specials/SpecialMovePage.php
+++ b/includes/specials/SpecialMovePage.php
@@ -1030,7 +1030,7 @@ class SpecialMovePage extends UnlistedSpecialPage {
                        //  having the i18n rebuilt for all deployments due to this security patch.
                        $out->addWikiTextAsInterface(
                                "The first $maximumMovedPages {{PLURAL:$maximumMovedPages|subpage|subpages}} " .
-                               ( $noSubpageMsg ? 'for the corresponding talk page' : 'for this page' ) . ' are shown below.'
+                               ( $noSubpageMsg ? 'for this page' : 'for the corresponding talk page' ) . ' are shown below.'
                        );
                } else {
                        $out->addWikiMsg( $wikiMsg, $this->getLanguage()->formatNum( $pagecount ) );

T357760-v2.patch4 KBDownload

+2

Dreamy_Jazz added a comment.Mar 27 2024, 5:46 PM
In T357760#9661968, @Reedy wrote:

Patch applies cleanly to MW-1.41-release and master.

rMWa99ec1b4fa59: Title: Use TitleArrayFromResult instead of TitleArray seems to cause the conflicts on MW-1.40-release

T357760-v2-REL1_40.patch4 KBDownload

And applying that patch with -3 works for MW-1.39-release (file rename)

T357760-v2-REL1_39.patch4 KBDownload

+2

gerritbot added a comment.Mar 28 2024, 10:44 PM

Change #1015410 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/core@REL1_39] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015410

gerritbot added a comment.Mar 28 2024, 10:44 PM

Change #1015415 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/core@REL1_40] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015415

gerritbot added a comment.Mar 28 2024, 10:45 PM

Change #1015419 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/core@REL1_41] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015419

gerritbot added a comment.Mar 28 2024, 10:51 PM

Change #1015423 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/core@master] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015423

gerritbot added a comment.Mar 28 2024, 10:57 PM

Change #1015410 merged by jenkins-bot:

[mediawiki/core@REL1_39] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015410

gerritbot added a comment.Mar 28 2024, 10:58 PM

Change #1015419 merged by jenkins-bot:

[mediawiki/core@REL1_41] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015419

gerritbot added a comment.Mar 28 2024, 10:59 PM

Change #1015415 merged by jenkins-bot:

[mediawiki/core@REL1_40] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015415

Reedy added a comment.Mar 28 2024, 11:04 PM

@Dreamy_Jazz The patches should be landing in master pretty soon. Would you mind making the followup to move the hardcoded en strings into proper i18n messages please?

No massive rush though!

gerritbot added a comment.Mar 28 2024, 11:20 PM

Change #1015423 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: Limit subpages displayed on Special:MovePage form

https://gerrit.wikimedia.org/r/1015423

Dreamy_Jazz added a comment.Mon, Apr 1, 7:54 PM

Can this be made public now?

Dreamy_Jazz added a comment.Mon, Apr 1, 7:55 PM
In T357760#9671830, @Reedy wrote:

@Dreamy_Jazz The patches should be landing in master pretty soon. Would you mind making the followup to move the hardcoded en strings into proper i18n messages please?

No massive rush though!

Sure. I'll do that now.

Reedy changed the visibility from "Custom Policy" to "Public (No Login Required)".Mon, Apr 1, 9:10 PM
Reedy changed the edit policy from "Custom Policy" to "All Users".
gerritbot added a comment.Mon, Apr 1, 9:56 PM

Change #1016011 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@master] Use i18n strings for truncated subpage message

https://gerrit.wikimedia.org/r/1016011

gerritbot added a comment.Mon, Apr 1, 10:09 PM

Change #1015684 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@REL1_41] Use i18n strings for truncated subpage message

https://gerrit.wikimedia.org/r/1015684

gerritbot added a comment.Mon, Apr 1, 10:10 PM

Change #1015685 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@REL1_40] Use i18n strings for truncated subpage message

https://gerrit.wikimedia.org/r/1015685

gerritbot added a comment.Mon, Apr 1, 10:10 PM

Change #1016026 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/core@REL1_39] Use i18n strings for truncated subpage message

https://gerrit.wikimedia.org/r/1016026

sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.Tue, Apr 2, 4:42 PM
sbassett added a project: SecTeam-Processed.
gerritbot added a comment.Sat, Apr 6, 2:18 AM

Change #1015684 merged by jenkins-bot:

[mediawiki/core@REL1_41] Use i18n strings for truncated subpage message in SpecialMovePage

https://gerrit.wikimedia.org/r/1015684

gerritbot added a comment.Sat, Apr 6, 2:18 AM

Change #1015685 merged by jenkins-bot:

[mediawiki/core@REL1_40] Use i18n strings for truncated subpage message in SpecialMovePage

https://gerrit.wikimedia.org/r/1015685

gerritbot added a comment.Sat, Apr 6, 2:25 AM

Change #1016011 merged by jenkins-bot:

[mediawiki/core@master] Use i18n strings for truncated subpage message in SpecialMovePage

https://gerrit.wikimedia.org/r/1016011

gerritbot added a comment.Sat, Apr 6, 2:56 AM

Change #1016026 merged by jenkins-bot:

[mediawiki/core@REL1_39] Use i18n strings for truncated subpage message in SpecialMovePage

https://gerrit.wikimedia.org/r/1016026

Reedy closed this task as Resolved.Sat, Apr 6, 3:00 AM
Reedy assigned this task to Dreamy_Jazz.
ReleaseTaggerBot added projects: MW-1.39-notes, MW-1.40-notes, MW-1.41-notes, MW-1.42-notes (1.42.0-wmf.26; 2024-04-09).Sat, Apr 6, 3:00 AM
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