Page MenuHomePhabricator

MassEditRegex is Vulnerable to CSRF Attacks (CVE-2021-46147)
Closed, ResolvedPublicSecurity

Description

MassEditRegex is currently not checking for CSRF tokens making it vulnerable to CSRF attacks.

Event Timeline

Here is a patch but I am not sure how to get it through Gerrit. Does that need to be done by the Security Team even for non-Wikimedia-deployed extensions?

sbassett subscribed.

@R4356th - a few things:

  1. For Wikimedia-deployed skins and extensions, when we find a vulnerability like this, we create a protected task and post a patch (as you've done) so that we can privately deploy the patch to production and then disclose it with one of our proper MediaWiki security releases (recent: T285404) or supplemental announcements (recent: T285414). But this process likely isn't necessary for many skins and extensions used by non-Wikimedia/WMF operators of MediaWiki.
  2. What the Security-Team would suggest, prior to publicly disclosing an issue via gerrit, is for the skin/extension maintainers or discoverers of any vulnerabilities like this, to reach out to as many people as possible (tagging them on this task if they have Phabricator accounts would work) so they can code-review and then deploy this patch to their MediaWiki installations prior to public disclosure. Unfortunately, this can be a tedious process and sites like WikiApiary, while helpful, are not always representative of actual usage of a given skin or extension.
  3. If you and/or the maintainers of the MassEditRegex extension are happy with the patch above and with any efforts to pre-announce the issue to affected parties and have them patch their MediaWiki installations, then we can push this up to gerrit as a standard change set for review and merge. I'd be happy to help with that, but the basic steps would be:
    1. git clone "https://gerrit.wikimedia.org/r/mediawiki/extensions/MassEditRegex"
    2. git apply --check --whitespace=fix /path/to/patchfile // rebase/3-way merge if necessary
    3. git am < /path/to/patchfile
    4. git review -R // assuming you have git-review installed
  4. We can also track this for the next supplemental security release (T292236 - though currently private) and help procure a proper CVE, if so desired.
  1. What the Security-Team would suggest, prior to publicly disclosing an issue via gerrit, is for the skin/extension maintainers or discoverers of any vulnerabilities like this, to reach out to as many people as possible (tagging them on this task if they have Phabricator accounts would work) so they can code-review and then deploy this patch to their MediaWiki installations prior to public disclosure. Unfortunately, this can be a tedious process and sites like WikiApiary, while helpful, are not always representative of actual usage of a given skin or extension.

Reaching out to 202 wikis is, as you say, tedious and something I would be interested in skipping.

  1. If you and/or the maintainers of the MassEditRegex extension are happy with the patch above and with any efforts to pre-announce the issue to affected parties and have them patch their MediaWiki installations, then we can push this up to gerrit as a standard change set for review and merge. I'd be happy to help with that, but the basic steps would be:
    1. git clone "https://gerrit.wikimedia.org/r/mediawiki/extensions/MassEditRegex"
    2. git apply --check --whitespace=fix /path/to/patchfile // rebase/3-way merge if necessary
    3. git am < /path/to/patchfile
    4. git review -R // assuming you have git-review installed

Well, I have tested the patch but the author (whom I have added here) does not seem active and I do not have +2 access. Could you or someone else from the Security-Team please help with that?

  1. We can also track this for the next supplemental security release (T292236 - though currently private) and help procure a proper CVE, if so desired.

That would be great. :)

Hi all, definitely haven't touched this for a while so can't help much, but I've eyeballed the patch and it looks fine so I'd be happy to +2 it if I still can.

I can't easily test it at the moment as I only have the 1.35.4 LTS version running and the latest git commit limits the extension to >= 1.36, so not sure how backporting the patch to the LTS version is handled.

The extension has two methods for editing pages - a form POST (which is fixed by this patch) and API calls from client-side Javascript code. I presume these API calls aren't affected by the issue?

Hi all, definitely haven't touched this for a while so can't help much, but I've eyeballed the patch and it looks fine so I'd be happy to +2 it if I still can.

Thank you! I am going to let you take care of uploading the patch in that case to make sure the vuln is not exploited by the time the patch is merged.

I can't easily test it at the moment as I only have the 1.35.4 LTS version running and the latest git commit limits the extension to >= 1.36, so not sure how backporting the patch to the LTS version is handled.

That commit did not touch SpecialMassEditRegex.php. I do not think backporting would be difficult.

The extension has two methods for editing pages - a form POST (which is fixed by this patch) and API calls from client-side Javascript code. I presume these API calls aren't affected by the issue?

Yeah, I do not think the JS API can be exploited.

Hi all, definitely haven't touched this for a while so can't help much, but I've eyeballed the patch and it looks fine so I'd be happy to +2 it if I still can.

Thank you! I am going to let you take care of uploading the patch in that case to make sure the vuln is not exploited by the time the patch is merged.

Did you still want to get a change set up to gerrit for review? Looks like I have +2 on that repo, so I could merge it if it passes CI and someone +1s it as confirmed testing and working for master. Let me know.

Please do if you can, I'm not very familiar with Gerrit etc. so it would be good to have someone who knows what they're doing involved!

Did you still want to get a change set up to gerrit for review? Looks like I have +2 on that repo, so I could merge it if it passes CI and someone +1s it as confirmed testing and working for master. Let me know.

Yes, that would be great! I have locally confirmed the patch works.

sbassett triaged this task as Medium priority.Oct 22 2021, 8:44 PM
sbassett changed Author Affiliation from N/A to Wikimedia Communities.
sbassett changed Risk Rating from N/A to Medium.
sbassett added a subscriber: gerritbot.

Change 733067 had a related patch set uploaded (by SBassett; author: R4356thwiki):

[mediawiki/extensions/MassEditRegex@master] [SECURITY] Add anti-CSRF mitigations for form submissions

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

Patch for master is up for review (see above). And thus, this issue has been publicly disclosed, per previous discussions from the ostensible project maintainers. We can theoretically make this task public as well, if that is desired right now. Once the gerrit change set is reviewed and merged, we can attempt any necessary backports and request a CVE as part of the next supplemental security release (T292236), due out later this coming December.

Change 733067 merged by jenkins-bot:

[mediawiki/extensions/MassEditRegex@master] [SECURITY] Add anti-CSRF mitigations for form submissions

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

I gave it a +2 and it looks like it has been merged now.

Change 733114 had a related patch set uploaded (by SBassett; author: R4356thwiki):

[mediawiki/extensions/MassEditRegex@REL1_37] [SECURITY] Add anti-CSRF mitigations for form submissions

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

Change 733114 merged by jenkins-bot:

[mediawiki/extensions/MassEditRegex@REL1_37] [SECURITY] Add anti-CSRF mitigations for form submissions

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

mmartorana renamed this task from MassEditRegex is Vulnerable to CSRF Attacks to MassEditRegex is Vulnerable to CSRF Attacks (CVE-2021-46147).Jan 10 2022, 5:02 PM
mmartorana changed the visibility from "Custom Policy" to "Public (No Login Required)".Jan 10 2022, 6:11 PM
mmartorana changed the edit policy from "Custom Policy" to "All Users".