Page MenuHomePhabricator

AbuseFilter logs suppression deletions (CVE-2021-31546)
Closed, ResolvedPublic

Description

Steinsplitter reported to me on IRC that they suppressed a page on beta labs and the AbuseFilter logged it like a normal deletion (visible to me, a non OS at the time).

http://commons.wikimedia.beta.wmflabs.org/wiki/Special:AbuseLog/25130 is the AF entry, the log action is: 17:26, 15 August 2014 Steinsplitter (talk | contribs | block) suppressed page GWToolset:BLAHAHA (testing abf) (view/restore)

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:34 AM
bzimport set Reference to bz69617.
bzimport changed Security from none to Software security bug.
Restricted Application changed the visibility from "Public (No Login Required)" to "acl*security (Project)". · View Herald TranscriptNov 22 2014, 3:34 AM
Restricted Application changed the edit policy from "All Users" to "acl*security (Project)". · View Herald Transcript

My simple-ish idea of how to fix this is pass $suppress to the ArticleDelete hook, and in AF just don't try to filter it if $suppress === true.

(In reply to Kunal Mehta (Legoktm) from comment #1)

My simple-ish idea of how to fix this is pass $suppress to the ArticleDelete
hook, and in AF just don't try to filter it if $suppress === true.

That seems reasonable. I'm surprised we don't already pass it.

(In reply to Chris Steipp from comment #3)

(In reply to Kunal Mehta (Legoktm) from comment #1)

My simple-ish idea of how to fix this is pass $suppress to the ArticleDelete
hook, and in AF just don't try to filter it if $suppress === true.

That seems reasonable. I'm surprised we don't already pass it.

Except we probably do want to still filter it, just additionally suppress any logs.

We could also hook ArticleDeleteComplete and do the suppression of the log from there.

Restricted Application changed the visibility from "acl*security (Project)" to "Custom Policy". · View Herald TranscriptNov 24 2014, 9:27 PM
Restricted Application changed the edit policy from "acl*security (Project)" to "Custom Policy". · View Herald Transcript
dpatrick added a project: Vuln-Infoleak.
dpatrick raised the priority of this task from Medium to High.Aug 14 2015, 8:52 PM
dpatrick lowered the priority of this task from High to Medium.

For some reason I was looking at the ArticleDelete hook recently and wondered why there was no $suppress parameter provided (ugh, now I'm wondering why it was because it'd probably be useful to link here). Shall we just upload a patch to Gerrit to do that?

For some reason I was looking at the ArticleDelete hook recently and wondered why there was no $suppress parameter provided (ugh, now I'm wondering why it was because it'd probably be useful to link here). Shall we just upload a patch to Gerrit to do that?

I think that's fine.

Should probably check the other extensions using this hook to ensure nothing is being left exposed in suppression deletions

Daimona added a project: Patch-For-Review.
Daimona added a subscriber: Daimona.

@sbassett @Reedy @Urbanecm Anybody who would be willing to take a final look at T71617#6722431 and sec-deploy it? Thanks!

@sbassett @Reedy @Urbanecm Anybody who would be willing to take a final look at T71617#6722431 and sec-deploy it? Thanks!

+2, patch approved. I'll ping Daimona on Monday so we can deploy&test the patch together.

+2, patch approved. I'll ping Daimona on Monday so we can deploy&test the patch together.

Thanks - if you need any help, let me know.

Also - there's a handful of these kinds of lingering sec patches, unfortunately. These are the other ones for AF of which I am aware:

  1. T214067#5027051
  2. T152394#5535402
  3. T223654#6717718
  4. T272244#6753978

Deployed:

12:25 <Urbanecm> !log Deploy security patch for T71617
12:25 <+stashbot> Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log

@sbassett Moving to Incoming. Could you please do the final honors (backport mainly), please?

I'll do my best to look at the other waiting patches.

Deployed:

Thanks.

@sbassett Moving to Incoming. Could you please do the final honors (backport mainly), please?

I'm tracking this on the supplemental patch for now: T270466. I'll try to get to the backports (at least for master) and CVE this week.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 9 2021, 9:38 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".

Change 670302 had a related patch set uploaded (by SBassett; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] SECURITY: Don't filter suppressions

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

sbassett lowered the priority of this task from Medium to Low.Mar 9 2021, 9:47 PM

Change 670302 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] SECURITY: Don't filter suppressions

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

Change 678659 had a related patch set uploaded (by Reedy; author: Daimona Eaytoy):

[mediawiki/extensions/AbuseFilter@REL1_35] SECURITY: Don't filter suppressions

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

Change 678660 had a related patch set uploaded (by Reedy; author: Daimona Eaytoy):

[mediawiki/extensions/AbuseFilter@REL1_31] SECURITY: Don't filter suppressions

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

Change 678660 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_31] SECURITY: Don't filter suppressions

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

Change 678659 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_35] SECURITY: Don't filter suppressions

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

sbassett renamed this task from AbuseFilter logs suppression deletions to AbuseFilter logs suppression deletions (CVE-2021-31546).Apr 23 2021, 6:48 PM