Page MenuHomePhabricator
Create Task
Maniphest T327613

CVE-2023-29140: GrowthExperiments new impact module shows revdeleted edits
Closed, ResolvedPublicSecurity
Actions

Description

The impact module displays the article titles for a select few edits of the user (last, or most viewed). The old Impact module had a check for rev_deleted, but I forgot to add that when writing the equivalent code for the new module, so it might show edits for which the username has been hidden.

Details

Risk Rating
Low
Author Affiliation
WMF Product
SubjectRepoBranchLines +/-
SECURITY: Do not include edits with revdeleted user in NewImpactmediawiki/extensions/GrowthExperimentsmaster+2 -0
Customize query in gerrit

Related Objects

Event Timeline

Tgr created this task.Jan 23 2023, 5:30 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 23 2023, 5:30 AM
Tgr added projects: Growth-Team (Sprint 0 (Growth Team)), Vuln-Infoleak, GrowthExperiments-ImpactModule.Jan 23 2023, 5:30 AM
Tgr added a comment.Jan 23 2023, 5:34 AM

T327613-NewImpact-revdeleted-edits.patch1 KBDownload

Tgr moved this task from Incoming to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.Jan 23 2023, 5:35 AM
Urbanecm added subscribers: kostajh, • KMorgan-WMF, Sgs and 5 others.Jan 23 2023, 3:13 PM

Adding other Growth engineers (+my WMF self).

Restricted Application removed a subscriber: Urbanecm. · View Herald TranscriptJan 23 2023, 3:13 PM
Urbanecm_WMF added a comment.Jan 23 2023, 3:19 PM

The patch was unfortunately published in Gerrit (pushed a new version of it to fix that).

CR+2, patch looks good to me and fixes the issue.

sbassett subscribed.Jan 23 2023, 3:26 PM
In T327613#8549351, @Urbanecm_WMF wrote:

The patch was unfortunately published in Gerrit (pushed a new version of it to fix that).

CR+2, patch looks good to me and fixes the issue.

Ok, so the literal patch file appears to have been accidentally included with an unrelated change set? So this still needs a deploy, soon-ish, with your CR+2.

Urbanecm_WMF added a comment.EditedJan 23 2023, 3:51 PM
In T327613#8549381, @sbassett wrote:

Ok, so the literal patch file appears to have been accidentally included with an unrelated change set? So this still needs a deploy, soon-ish, with your CR+2.

Indeed. Deployed (SAL):

16:50 <urbanecm> !log Deploy security patch for T327613
16:50 <+stashbot> Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log
Urbanecm_WMF moved this task from Code Review to QA on the Growth-Team (Sprint 0 (Growth Team)) board.Jan 23 2023, 3:54 PM
Urbanecm_WMF assigned this task to Tgr.Jan 23 2023, 4:10 PM
Tgr added a comment.Jan 23 2023, 6:02 PM
In T327613#8549351, @Urbanecm_WMF wrote:

The patch was unfortunately published in Gerrit (pushed a new version of it to fix that).

Ugh, sorry. I tracked this down to a setting in PhpStorm that I wasn't aware of (When files are created / Add silently / Apply to files created outside PhpStorm).

sbassett mentioned this in T325849: Write and send supplementary release announcement for extensions and skins with security patches (1.35.10/1.38.6/1.39.3).Jan 23 2023, 6:11 PM
sbassett added a comment.Jan 23 2023, 6:15 PM

Hey all - thanks for the quick response and deployment. Tracking this issue for the next supplemental security release at T325849, but this can be opened up and backported in gerrit any time now.

sbassett moved this task from Incoming to Watching on the Security-Team board.Jan 23 2023, 6:16 PM
sbassett added a project: SecTeam-Processed.
Urbanecm added a subscriber: gerritbot.Jan 30 2023, 10:04 PM
Urbanecm_WMF added a comment.Jan 30 2023, 10:06 PM

Uploaded as https://gerrit.wikimedia.org/r/c/mediawiki/extensions/GrowthExperiments/+/885042; relevant feature's in no release branches, so just master.

gerritbot added a comment.Jan 30 2023, 10:24 PM

Change 885042 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] SECURITY: Do not include edits with revdeleted user in NewImpact

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

brennen subscribed.Feb 2 2023, 9:34 PM

Removed /srv/patches/1.40.0-wmf.21/extensions/GrowthExperiments/01-T327613.patch just now.

Etonkovidova closed this task as Resolved.Feb 10 2023, 1:45 AM

Checked for revdeleted in betalabs - works as expected: if a revision is deleted, the Impact module would not display the stats for pageviews.

sbassett added a comment.Feb 10 2023, 5:03 PM

@Etonkovidova @Urbanecm_WMF - I assume we're fine to make this task public since the lone backport to master already went through gerrit?

Urbanecm_WMF added a comment.Feb 11 2023, 6:33 AM

Yes, +1 to publishing from me!

sbassett triaged this task as Low priority.Feb 13 2023, 3:01 PM
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.
mmartorana renamed this task from GrowthExperiments new impact module shows revdeleted edits to CVE-2023-29140: GrowthExperiments new impact module shows revdeleted edits.Apr 3 2023, 3:19 PM
Urbanecm_WMF removed a subscriber: MShilova_WMF.Jul 31 2023, 6:04 PM
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