Page MenuHomePhabricator

CVE-2024-47913: abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details
Closed, ResolvedPublic2 Estimated Story PointsSecurity

Description

What is the problem?

For the API action abusefiltercheckmatch, I can check a pattern like user_unnamed_ip="1.2.3.4" against an AbuseFilter log entry that I don't have permission to see and, if I guess the right IP, it will return true. This applies more widely than protected filters, as this can be used to inspect the log details for logs created by private filters.

Although it requires you to guess IPs, you can use things like like and >, < in order to narrow it down.

This has been a problem since before REL1_19, as the code in rMRELb30697e94cec for the abusefiltercheckmatch API looks like it has the same problem of not checking the right.

Steps to reproduce problem
For protected filters
  1. Create an abuse filter with condition user_unnamed_ip.
  2. While logged out, perform an action like making an edit.
  3. Look in Special:AbuseLog to see the log for the action you made in step 2. Make a note of the log ID (which you will see in the URL of the links for details or examine).
  4. Login as an admin who does not have the abusefilter-access-protected-vars right.
  5. Go to Special:ApiSandbox#action=abusefiltercheckmatch&format=json&filter=user_unnamed_ip%3D%22%3Cip%3E%22.
  6. Fill out <ip> with the IP of the user from step 2. Fill out the logid parameter with the log ID from step 3. Submit.

Expected behaviour: The response from the API should say the user does not have the right to use protected variables.
Observed behaviour: A response:

{
    "abusefiltercheckmatch": {
        "result": true
    }
}
For private filters
  1. Create an abuse filter with the condition summary = 'should be private' and mark it as private
  2. Trigger the private filter by making an edit with the edit summary should be private
  3. Look in Special:AbuseLog to see the log for the action you made in step 2. Make a note of the log ID (which you will see in the URL of the links for details or examine).
  4. Login as an admin who does not have the abusefilter-log-detail right but has the abusefilter-modify right
  5. Go to Special:ApiSandbox#action=abusefiltercheckmatch&format=json.
  6. Set the filter argument as summary = 'should be private'
  7. Fill out the logid parameter with the log ID from step 3. Submit.

Expected behaviour: The response from the API should say the user does not have rights to see abusefilter log details
Observed behaviour: A response:

{
    "abusefiltercheckmatch": {
        "result": true
    }
}
Environment

Wiki(s): Abuse Filter – (f65ed2b) 07:22, 21 August 2024.

Event Timeline

There seems to be a wider problem of the logid parameter not being checked for user permissions. Using the CheckMatch API, everyone who can use the API can inspect any abuselog entry that they would normally be allowed to see. The only checks are on the visibility of the log entry itself (abuse_filter_log.afl_deleted) and of the associated revision. To reproduce:

  • Find any AbuseLog entry (the filter can be public, private, or with protected variables; it doesn't matter)
  • Remove the abusefilter-log-detail right from sysops
  • Test the abusefiltercheckmatch API using the logid from above

You're allowed to check arbitrary rules against the log entry, which means you can extrapolate all the variables.

I think the solution would be the same for both issues: apply proper permission checks as done in SpecialAbuseLog and ApiQueryAbuseLog. One thing worth mentioning is that the whole AbuseLog area wasn't improved that much in the refactoring that happened a few years ago, and there are no dedicated entities/services/methods for accessing or interacting with AbuseLog entries.

JayCano set the point value for this task to 2.Aug 26 2024, 10:23 AM

Reviewed the patch and it lgtm. Tested it against protected and private filters and confirmed both will disallow checking matches on filters the user shouldn't have access to.

Patch deployed. I did encounter a problem where the patch file did not get added to /srv/patches due to the directory not existing. I fixed this and manually added the patch file, so should be all good.

Patch deployed. I did encounter a problem where the patch file did not get added to /srv/patches due to the directory not existing. I fixed this and manually added the patch file, so should be all good.

Er, that's probably not good, as I think there were still two deployed security patches (T355073, T366991) which hadn't been released yet... I guess we'll need to investigate that.

Oh wait, do you just mean the /srv/patches/1.43.0-wmf.23 hadn't been created?

To clarify, I mean /srv/patches/1.43.0-wmf.22/extensions/AbuseFilter and /srv/patches/1.43.0-wmf.23/extensions/AbuseFilter.

Ah, ok, yeah everything looks fine on deployment. Sorry for the confusion. We have, in the past, had directories/patches randomly disappear under /srv/patches.

sbassett added a parent task: Restricted Task.Sep 17 2024, 3:49 PM

So... As Temporary accounts are basically new functionality in 1.43 (which isn't released), this doesn't actually need holding for the security release, nor does it need to be part of it...

So... As Temporary accounts are basically new functionality in 1.43 (which isn't released), this doesn't actually need holding for the security release, nor does it need to be part of it...

No, because the issue applies more widely than just temporary account functionality, because of the following comment:

  • Find any AbuseLog entry (the filter can be public, private, or with protected variables; it doesn't matter)

As such, the issue exists in 1.42 and before.

Can someone please make sure the task title and description are fully up to date then please?

It's hard to write/request a CVE without the full details, and similarly having an idea of how long this has been an issue for (even if "probably forever").

Can someone please make sure the task title and description are fully up to date then please?

It's hard to write/request a CVE without the full details, and similarly having an idea of how long this has been an issue for (even if "probably forever").

Sure. I can do that. done

Dreamy_Jazz renamed this task from abusefiltercheckmatch can reveal IP of a temporary user to abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details.Sep 25 2024, 1:44 PM
Dreamy_Jazz updated the task description. (Show Details)
Reedy renamed this task from abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details to CVE-2024-: abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details.Sep 25 2024, 1:50 PM

<3 for the updates!

Proposed patch:

Some trivial poking around needed to get this patch onto MW-1.42-release

It then trivially backports to MW-1.41-release, it would obviously apply to MW-1.40-release but that's (recently) EOL (so probably worth calling out in the announcement), and it applies to MW-1.39-release

In 1.39, Title isn't namespaced, so from a cursory glance, that is probably the only difference between them... Everything else seems to be in AF, and has been around for a while.

Change #1076852 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/extensions/AbuseFilter@REL1_39] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076853 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/extensions/AbuseFilter@REL1_41] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076854 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/extensions/AbuseFilter@REL1_42] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076855 had a related patch set uploaded (by Reedy; author: Dreamy Jazz):

[mediawiki/extensions/AbuseFilter@master] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Everything else seems to be in AF, and has been around for a while.

Or not.

rEABFca23e9f06b53: Convert `af_hidden` into a bitmask

So all the patches will actually break things...

Change #1076854 merged by Reedy:

[mediawiki/extensions/AbuseFilter@REL1_42] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076853 merged by Reedy:

[mediawiki/extensions/AbuseFilter@REL1_41] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076852 merged by Reedy:

[mediawiki/extensions/AbuseFilter@REL1_39] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076863 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/AbuseFilter@REL1_42] Fixup T372998 backport

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

Change #1076865 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/AbuseFilter@REL1_41] Fixup T372998 backport

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

Change #1076866 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/AbuseFilter@REL1_39] Fixup T372998 backport

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

Change #1076863 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_42] Fixup T372998 backport

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

Change #1076855 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] SECURITY: abusefiltercheckmatch: Check if user can see log details

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

Change #1076865 merged by Reedy:

[mediawiki/extensions/AbuseFilter@REL1_41] Fixup T372998 backport

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

Change #1076866 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_39] Fixup T372998 backport

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

Reedy renamed this task from CVE-2024-: abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details to CVE-2024-47913: abusefiltercheckmatch does not check the user for the abusefilter-log-detail right before matching against log details.Oct 4 2024, 9:53 PM
sbassett triaged this task as Medium priority.Feb 24 2025, 9:44 PM
sbassett changed Author Affiliation from N/A to WMF Technology.
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 Medium.

Change #1127079 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/AbuseFilter@master] Replace AbuseFilterServices with DI in CheckMatch API

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

Change #1127079 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] Use dependency injection over AbuseFilterServices in CheckMatch API

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