Page MenuHomePhabricator
Create Task
Maniphest T104807

Older hidden versions of a currently-public AbuseFilter are exposed via diffs (CVE-2019-18612)
Closed, ResolvedPublic
Actions

Assigned To
Daimona
Authored By
Luke081515
Jul 5 2015, 3:55 PM
Tags
Referenced Files
F30872887: T104807.patch
Oct 21 2019, 1:41 PM
F30864315: T104807.patch
Oct 19 2019, 6:07 PM
F30864146: T104807.patch
Oct 19 2019, 5:25 PM
F27938263: T104807.patch
Jan 18 2019, 12:27 PM
Subscribers
Aklapper
Ayokura
Daimona
hoo
Krenair
Luke081515
matej_suchanek
View All 11 Subscribers

Description

There is a problem with hidden abusefilters, for example: this filter is actually not hidden: https://de.wikipedia.org/wiki/Spezial:Missbrauchsfilter/3, but older versions are hidden, for example, the last hidden version: https://de.wikipedia.org/wiki/Spezial:Missbrauchsfilter/history/3/item/40, without sysop rights not visible. The version after this is not hidden, so visible for all users: https://de.wikipedia.org/wiki/Spezial:Missbrauchsfilter/history/3/item/42. The problem now is, that the difference is visible for all users: https://de.wikipedia.org/wiki/Spezial:Missbrauchsfilter/history/3/diff/prev/42, and also the difference beetween hidden versions: https://de.wikipedia.org/wiki/Spezial:Missbrauchsfilter/history/3/diff/prev/40, so non sysops can construct the hidden version, thats the problem.

Details

SubjectRepoBranchLines +/-
SECURITY: Check visibility for each version in ViewDiffmediawiki/extensions/AbuseFilterREL1_32+37 -13
SECURITY: Check visibility for each version in ViewDiffmediawiki/extensions/AbuseFilterREL1_31+39 -15
SECURITY: Check visibility for each version in ViewDiffmediawiki/extensions/AbuseFilterREL1_33+37 -13
SECURITY: Check visibility for each version in ViewDiffmediawiki/extensions/AbuseFilterREL1_34+37 -13
SECURITY: Check visibility for each version in ViewDiffmediawiki/extensions/AbuseFiltermaster+37 -13
Customize query in gerrit

Related Objects

Event Timeline

Maniphest changed the visibility from "Public (No Login Required)" to "Custom Policy".Jul 5 2015, 3:55 PM
Maniphest changed the edit policy from "All Users" to "Custom Policy".
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 5 2015, 3:55 PM
Luke081515 created this task.Jul 5 2015, 3:55 PM
Luke081515 triaged this task as High priority.
Luke081515 updated the task description. (Show Details)
Luke081515 added projects: AbuseFilter, WMF-NDA.
Luke081515 changed Security from None to Other confidential issue.
Luke081515 edited subscribers, added: Luke081515; removed: Aklapper.
Krenair changed Security from Other confidential issue to Software security bug.Jul 5 2015, 3:57 PM
Restricted Application added a project: acl*security. · View Herald TranscriptJul 5 2015, 3:57 PM
Krenair removed a project: WMF-NDA.Jul 5 2015, 3:57 PM
Krenair changed the visibility from "Custom Policy" to "Custom Policy".
Krenair changed the edit policy from "Custom Policy" to "Custom Policy".
Krenair renamed this task from Problem with hidden AbuseFilters, not completly hidden, difference is visible to Older hidden versions of a currently-public AbuseFilter are exposed via diffs.Jul 5 2015, 7:44 PM
Krenair added projects: Security-Extensions, Vuln-Infoleak.
Krenair subscribed.
hoo subscribed.Jul 5 2015, 8:32 PM
Krenair merged a task: Restricted Task.Feb 23 2016, 3:24 AM
Krenair added subscribers: Se4598, Ayokura, Aklapper.
Krenair changed the visibility from "Custom Policy" to "Custom Policy".Feb 24 2016, 11:13 AM
Krenair changed the edit policy from "Custom Policy" to "Custom Policy".
Daimona claimed this task.Jan 18 2019, 12:27 PM
Daimona subscribed.

T104807.patch3 KBDownload

To fix this (fairly simple) and avoid showing diff links.

Urbanecm moved this task from Backlog / Other to Patch pending review on the acl*security board.Oct 19 2019, 4:52 PM
Urbanecm added a subscriber: matej_suchanek.
Urbanecm subscribed.Oct 19 2019, 5:20 PM
Urbanecm added a comment.Oct 19 2019, 5:25 PM

T104807.patch3 KBDownload

Fixed conflict with newer version of code, confirmed it works locally.

Daimona added a comment.Oct 19 2019, 6:07 PM
In T104807#5588698, @Urbanecm wrote:

T104807.patch3 KBDownload

Fixed conflict with newer version of code, confirmed it works locally.

A piece was missed there, which I restored in the patch below. Plus: use canViewPrivate everywhere, order by afh_id in a query, and add a @todo about caching for when the code will be public.

T104807.patch3 KBDownload

Urbanecm added a comment.Oct 19 2019, 9:24 PM

Thank you Daimona. Actions are always public, so it is not necessary to hide them, I think.

Daimona added a comment.Oct 20 2019, 9:35 AM
In T104807#5588863, @Urbanecm wrote:

Thank you Daimona. Actions are always public, so it is not necessary to hide them, I think.

I wouldn't be too sure about that (e.g. throttle params are a bit sensitive), but I agree that it's probably fine not to hide them. Anyway, we can think about it once the code is public.

Daimona added a comment.Oct 21 2019, 1:41 PM

T104807.patch3 KBDownload

Daimona added a comment.Oct 21 2019, 1:49 PM

Note: when viewing the diff right before the hidden version, you'll still see a "newer change" button at the bottom; clicking it will just tell you that you cannot see the revision. I guess we can make it disappear in a follow-up.

Daimona added subscribers: sbassett, Reedy.Oct 21 2019, 1:54 PM

@sbassett, @Reedy Waiting for your green light before pushing publicly :)

Urbanecm moved this task from Patch pending review to Pending deployment / release on the acl*security board.Oct 21 2019, 9:33 PM
In T104807#5591681, @Daimona wrote:

T104807.patch3 KBDownload

Deployed earlier today as-of this patch.

sbassett mentioned this in T234983: Write and send supplementary release announcement for extensions and skins with security patches (MediaWiki 1.31.6/1.32.6/1.33.2).Oct 21 2019, 10:00 PM
In T104807#5593174, @Urbanecm wrote:

Deployed earlier today as-of this patch.

Thanks.

In T104807#5591714, @Daimona wrote:

@sbassett, @Reedy Waiting for your green light before pushing publicly :)

@Daimona @Urbanecm - can we give this at least a couple of days in production to assess functionality and stability? Then we can do the backports through gerrit. Also tracking in T234983 and will plan to request a CVE soon, though that shouldn't hold up the backports.

Urbanecm added a comment.Oct 22 2019, 5:49 AM
In T104807#5593253, @sbassett wrote:
In T104807#5593174, @Urbanecm wrote:

Deployed earlier today as-of this patch.

Thanks.

In T104807#5591714, @Daimona wrote:

@sbassett, @Reedy Waiting for your green light before pushing publicly :)

@Daimona @Urbanecm - can we give this at least a couple of days in production to assess functionality and stability? Then we can do the backports through gerrit. Also tracking in T234983 and will plan to request a CVE soon, though that shouldn't hold up the backports.

Sure!

Daimona added a comment.Oct 22 2019, 7:58 AM
In T104807#5593253, @sbassett wrote:

@Daimona @Urbanecm - can we give this at least a couple of days in production to assess functionality and stability? Then we can do the backports through gerrit. Also tracking in T234983 and will plan to request a CVE soon, though that shouldn't hold up the backports.

Sure, thanks. Can we also make this task public once the backports are on gerrit?

Urbanecm added a comment.Oct 22 2019, 3:54 PM

i guess so, given no sensitive things are here.

sbassett added a comment.Oct 22 2019, 6:37 PM
In T104807#5593937, @Daimona wrote:

Sure, thanks. Can we also make this task public once the backports are on gerrit?

In T104807#5595805, @Urbanecm wrote:

i guess so, given no sensitive things are here.

I'm not seeing anything on the task that would prevent it from being made public. And given that it's for a non-bundled extension, we don't have to hold the task for a mediawiki security release or anything like that. The new announcements I've been trying to do (e.g. T234983) are designed to occur after backports and any other public disclosure (CVE, etc.)

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 28 2019, 8:33 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".
gerritbot added a comment.Oct 28 2019, 8:33 PM

Change 546723 had a related patch set uploaded (by SBassett; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@master] SECURITY: Check visibility for each version in ViewDiff

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

gerritbot added a project: Patch-For-Review.Oct 28 2019, 8:33 PM

Change 546726 had a related patch set uploaded (by SBassett; owner: Daimona Eaytoy):
[mediawiki/extensions/AbuseFilter@REL1_34] SECURITY: Check visibility for each version in ViewDiff

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

sbassett added a comment.Oct 28 2019, 8:49 PM

Ugh, security backports conflicting with https://gerrit.wikimedia.org/r/480069.

gerritbot added a comment.Oct 28 2019, 9:31 PM

Change 546723 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] SECURITY: Check visibility for each version in ViewDiff

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

gerritbot added a comment.Oct 28 2019, 9:41 PM

Change 546726 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_34] SECURITY: Check visibility for each version in ViewDiff

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

sbassett lowered the priority of this task from High to Medium.Oct 28 2019, 9:50 PM
sbassett added a comment.EditedOct 28 2019, 9:57 PM

Backports for master and REL1_34 were merged, CVE requested. It appears that for the other supported release branches (1.31, 1.32, 1.33) it's just an issue of line differences for the patch. I'll try to get some additional patches uploaded to gerrit for these branches. @Daimona - would you be able to review those just in case the patch isn't relevant? (I think it still is - whoops, it's not.)

gerritbot added a comment.Oct 28 2019, 9:59 PM

Change 546752 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/AbuseFilter@REL1_31] SECURITY: Check visibility for each version in ViewDiff

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

ReleaseTaggerBot added a project: MW-1.35-notes (1.35.0-wmf.4; 2019-10-29).Oct 28 2019, 10:00 PM
gerritbot added a comment.Oct 28 2019, 10:01 PM

Change 546753 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/AbuseFilter@REL1_32] SECURITY: Check visibility for each version in ViewDiff

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

gerritbot added a comment.Oct 28 2019, 10:03 PM

Change 546754 had a related patch set uploaded (by SBassett; owner: SBassett):
[mediawiki/extensions/AbuseFilter@REL1_33] SECURITY: Check visibility for each version in ViewDiff

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

gerritbot added a comment.Oct 29 2019, 4:59 AM

Change 546754 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_33] SECURITY: Check visibility for each version in ViewDiff

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

sbassett renamed this task from Older hidden versions of a currently-public AbuseFilter are exposed via diffs to Older hidden versions of a currently-public AbuseFilter are exposed via diffs (CVE-2019-18612).Oct 29 2019, 3:51 PM
Daimona added a comment.Oct 29 2019, 4:58 PM

FTR: I've updated the backports for 1.31, 32, and 33 to avoid using AbuseFilter::canViewPrivate (introduced in 1.34). For the 1.31 one, I also had to upgrade PHPCS to 19.1.0 (and disable a new sniff) because older versions fail on PHP7.3 (T233759).

As I've already pointed out, things would've been easier with phan running on REL branches (T226945 / T231966).

sbassett added a comment.Oct 29 2019, 5:22 PM

@Daimona - great, thanks for the help! Once the remaining backports are merged, I'll resolve this task.

gerritbot added a comment.Oct 31 2019, 4:01 PM

Change 546752 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_31] SECURITY: Check visibility for each version in ViewDiff

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

gerritbot added a comment.Oct 31 2019, 4:02 PM

Change 546753 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@REL1_32] SECURITY: Check visibility for each version in ViewDiff

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

sbassett closed this task as Resolved.Oct 31 2019, 4:05 PM
sbassett removed a project: Patch-For-Review.
• chasemp added a project: Security.Feb 10 2020, 10:59 PM
• chasemp removed a project: acl*security.Feb 20 2020, 8:17 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