Page MenuHomePhabricator
Create Task
Maniphest T95589

Users with viewsuppressed but not suppressrevision can remove suppression
Closed, ResolvedPublic
Actions

Assigned To
• csteipp
Authored By
Anomie
Apr 9 2015, 5:48 PM
Tags
Referenced Files
F2558076: T91205_1.26wmf22.patch
Oct 16 2015, 4:26 PM
F2724606: T91205-REL1_24.patch
Oct 16 2015, 4:26 PM
F2724580: T91205-REL1_25.patch
Oct 16 2015, 4:26 PM
F110631: 0001-SECURITY-RevDel-Check-all-revisions-for-suppression-.patch
Apr 9 2015, 6:07 PM
Subscribers
Aklapper
• csteipp
• demon
Ejegg
gerritbot
Grunny
Jalexander
View All 9 Subscribers

Description

To reproduce:

  • Revision-delete a revision with suppression.
  • As a user with viewsuppressed but not suppressrevision, construct a url to action=revisiondelete that includes both the suppressed revision and any newer non-suppressed revision.
    • You will be presented with the usual "change" form.
  • Submit the form. The revision-deletion settings on the suppressed revision will be changed, including removal of the suppression.

patch:

0001-SECURITY-RevDel-Check-all-revisions-for-suppression-.patch2 KBDownload

  • 1.26 - same as master (
    T91205_1.26wmf22.patch8 KBDownload
    )
  • 1.25 -
    T91205-REL1_25.patch8 KBDownload
  • 1.24 -
    T91205-REL1_24.patch8 KBDownload

affected versions: 1.24.0 - 1.25.2
type: Missing Authorization (CWE-862)
CVE: CVE-2015-8004

Details

SubjectRepoBranchLines +/-
SECURITY: RevDel: Check all revisions for suppression, not just the firstmediawiki/coremaster+20 -2
SECURITY: RevDel: Check all revisions for suppression, not just the firstmediawiki/coreREL1_26+20 -2
SECURITY: RevDel: Check all revisions for suppression, not just the firstmediawiki/coreREL1_24+20 -2
SECURITY: RevDel: Check all revisions for suppression, not just the firstmediawiki/coreREL1_25+20 -2
Customize query in gerrit

Related Objects
Search...

Event Timeline

Anomie created this task.Apr 9 2015, 5:48 PM
Anomie raised the priority of this task from to Needs Triage.
Anomie updated the task description. (Show Details)
Anomie added projects: MediaWiki-Revision-deletion, acl*security.
Anomie changed the visibility from "Public (No Login Required)" to "Custom Policy".
Anomie changed the edit policy from "All Users" to "Custom Policy".
Anomie changed Security from None to Software security bug.
Anomie subscribed.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 9 2015, 5:48 PM
Anomie added a comment.Apr 9 2015, 5:50 PM

At a quick glance, no local groups on WMF wikis seem to be granted this right. Global groups 'steward' and 'founder' have it.

Anomie updated the task description. (Show Details)Apr 9 2015, 5:53 PM
• csteipp subscribed.Apr 9 2015, 6:00 PM

I'm assuming we're not rechecking the user right when we actually do the unsuppress?

Anomie added a project: Patch-For-Review.Apr 9 2015, 6:07 PM

0001-SECURITY-RevDel-Check-all-revisions-for-suppression-.patch2 KBDownload

Anomie added a comment.Apr 9 2015, 6:08 PM

What's going on is that it only checks the first revision's suppression status when doing a $isSuppressed && !$user->isAllowed( 'suppressrevision' )-style check for deciding whether to allow changing.

Krenair subscribed.Apr 9 2015, 6:18 PM

Looks good to me.

• csteipp added a comment.Apr 9 2015, 6:28 PM

+1

@Krenair, could you deploy this during swat today?

Krenair added a comment.Apr 9 2015, 6:42 PM

Is there any point deploying this to WMF wikis? It sounds like the only people who could exploit it there are stewards (who can give themselves whatever right they want anyway) and Jimbo Wales.

Jalexander subscribed.Apr 9 2015, 11:01 PM
In T95589#1195547, @Krenair wrote:

Is there any point deploying this to WMF wikis? It sounds like the only people who could exploit it there are stewards (who can give themselves whatever right they want anyway) and Jimbo Wales.

I would say yes, I can let Jimmy and the Stewards (more importantly the Stewards) know about it. My guess is they never used that as an unexpected work flow but if the stewards need the ability they can add the appropriate right. In the end we never know if some researcher right or other right might be created that lets people abuse/use this bug and so closing it on the wiki as soon as possible is always smart even if the abuse vector is small.

Krenair added a comment.Apr 10 2015, 12:03 AM

Okay, deployed. I wonder if Jimmy has ever signed a WMF NDA or provided identification.

Jalexander added a comment.Apr 10 2015, 12:33 AM
In T95589#1196491, @Krenair wrote:

Okay, deployed. I wonder if Jimmy has ever signed a WMF NDA or provided identification.

The NDA I can answer, the board has to do it on a routine basis so he's likely signed multiple of them ;) I also think we just have his ID on file at this point.

• dpatrick closed this task as Resolved.Aug 11 2015, 9:04 PM
• dpatrick triaged this task as Low priority.
• csteipp reopened this task as Open.Aug 11 2015, 9:04 PM
• csteipp added a parent task: T108064: MediaWiki Security release 1.25.3.
• csteipp added a project: Vuln-MissingAuthz.Aug 20 2015, 9:30 PM
• csteipp closed this task as Resolved.Sep 8 2015, 5:39 PM
• csteipp claimed this task.

I think that was an accidental reopen. This is patched on the cluster, should be released.

• demon subscribed.Oct 15 2015, 8:52 PM

0001-SECURITY-RevDel-Check-all-revisions-for-suppression-.patch2 KBDownload
applies to all branches REL1_24 and above. Will work on REL1_23 patch.

• demon added a comment.Oct 15 2015, 8:59 PM

Actually, 1.23 doesn't have viewsuppressed, so it won't get this patch.

• csteipp added a subscriber: Grunny.Oct 16 2015, 3:03 PM
• csteipp added a subscriber: ProgramCeltic.Oct 16 2015, 4:09 PM
• csteipp updated the task description. (Show Details)Oct 16 2015, 4:26 PM
• csteipp added a subscriber: Ejegg.Oct 16 2015, 4:38 PM
• demon changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 16 2015, 6:12 PM
• demon changed the edit policy from "Custom Policy" to "All Users".
• demon changed Security from Software security bug to None.
gerritbot subscribed.Oct 16 2015, 6:19 PM

Change 246870 had a related patch set uploaded (by Chad):
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 6:19 PM

Change 246875 had a related patch set uploaded (by Chad):
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 6:24 PM

Change 246886 had a related patch set uploaded (by Chad):
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 7:03 PM

Change 246870 merged by jenkins-bot:
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 7:57 PM

Change 246875 merged by jenkins-bot:
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

ReleaseTaggerBot added projects: MW-1.25-release, MW-1.24-release.Oct 16 2015, 8:00 PM
gerritbot added a comment.Oct 16 2015, 8:56 PM

Change 246975 had a related patch set uploaded (by Chad):
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 9:26 PM

Change 246975 merged by jenkins-bot:
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

gerritbot added a comment.Oct 16 2015, 9:45 PM

Change 246886 merged by jenkins-bot:
SECURITY: RevDel: Check all revisions for suppression, not just the first

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

ReleaseTaggerBot added projects: MW-1.27-release (WMF-deploy-2015-10-27_(1.27.0-wmf.4)), MW-1.27-release-notes, MW-1.26-release.Oct 16 2015, 10:00 PM
• csteipp updated the task description. (Show Details)Nov 3 2015, 9:12 PM
• chasemp added a project: Security.Feb 10 2020, 11:00 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:18 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:43 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