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

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:

  • 1.26 - same as master ()
  • 1.25 -
  • 1.24 -

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

Anomie created this task.Apr 9 2015, 5:48 PM
Anomie updated the task description. (Show Details)
Anomie raised the priority of this task from to Needs Triage.
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 added a subscriber: Anomie.
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 added a subscriber: csteipp.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 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 added a subscriber: Krenair.Apr 9 2015, 6:18 PM

Looks good to me.

+1

@Krenair, could you deploy this during swat today?

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.

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.

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

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 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 added a subscriber: demon.Oct 15 2015, 8:52 PM

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 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.

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

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

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

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

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

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

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

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

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

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

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

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

csteipp updated the task description. (Show Details)Nov 3 2015, 9:12 PM