Page MenuHomePhabricator

action=edit&redirect=true bypasses protection
Closed, ResolvedPublicSecurity

Description

List of steps to reproduce (step by step, including full links if applicable):
*Steps to reproduce not yet available, example of recent occurrence below:

On the English Wikipedia, a edit to a sysop-protected page was made, despite the editor not having the editprotected or protect rights
*Edit in question: https://en.wikipedia.org/w/index.php?title=User%3AQst&type=revision&diff=1088768331&oldid=981612139
*On page: https://en.wikipedia.org/wiki/User:Qst
*By User: https://en.wikipedia.org/wiki/User:SDZeroBot
*Examine output: https://en.wikipedia.org/wiki/Special:AbuseFilter/examine/1507548235

What happens?:
*The edit was allowed.

What should have happened instead?":
*The edit should have been disallowed

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc.:

Event Timeline

Urbanecm set Security to Software security bug.May 23 2022, 3:00 PM
Urbanecm added projects: Security, Security-Team.
Urbanecm changed the visibility from "Public (No Login Required)" to "Custom Policy".
Urbanecm changed the subtype of this task from "Bug Report" to "Security Issue".
Urbanecm added a subscriber: Urbanecm.

this is a security bug

Just realized, I opened T309031 as the security report for the issue in general

Just realized, I opened T309031 as the security report for the issue in general

FYI, there's "promote to security issue" in the top-right menu you can click to fix those kind of mistakes.

taavi triaged this task as High priority.

Bringing info across from duplicates T309030 and T309031 - this is an API edit request through a redirect.

Given:

An API call by a non-admin user as such:

  • action=edit
  • redirect=true
  • title=User:Stwalkerster/sandbox2
  • appendtext=Test edit
  • summary=test

The fully-protected page is edited by a non-admin. See https://en.wikipedia.org/w/index.php?title=User:Stwalkerster/sandbox&action=history

Proposed patch:

taavi renamed this task from edit allowed despite protection to action=edit&redirect=true bypasses protection.May 23 2022, 3:08 PM
taavi added a project: MediaWiki-Action-API.
In T309028#7950254, @Majavah wrote:

Proposed patch:

+2 approved, happy to test during deployment if you want

Noting here that this was posted on a public forum :( https://en.wikipedia.org/wiki/Wikipedia:Village_pump_(technical)#Edit_to_User:Qst

In T309028#7950254, @Majavah wrote:

Proposed patch:

Deployed (SAL) after this was approved by @Urbanecm on _security.

+2 approved, happy to test during deployment if you want

Thanks for the offer, but we already deployed this.

Root Cause? Was this a recent regression? Can't imagine this has been a long-standing bug that was just discovered.

Root Cause? Was this a recent regression? Can't imagine this has been a long-standing bug that was just discovered.

I believe this is from T290639: Move redirect lookup logic into a service object and rMW8fe9e0317f2c: Introduce `Redirect(Lookup&Store)` services to handle redirects - previously the target of a redirect always updated $titleObj
We should include this in 1.38 before its released

Could someone check that 8fe9e0317f2c didn't introduce any similar bugs to EditPage please?

In T309028#7950467, @Majavah wrote:

Could someone check that 8fe9e0317f2c didn't introduce any similar bugs to EditPage please?

{{looking}}
Appears to have introduced a minor bug where ContentTransformer::preloadTransform() will be called for the wrong title, but nothing that bypasses protection - edit page doesn't care if a page is a redirect, you can still edit it, but the api will try to edit the redirect target instead. Is it okay to send the ContentTransformer fix on gerrit? Or would that raise flags?

Just checking, from a deployment perspective is this currently remediated on WMF wikis? Tests seem to be passing on testwiki now.

Just checking, from a deployment perspective is this currently remediated on WMF wikis? Tests seem to be passing on testwiki now.

(just in case you didn't get an answer here @Xaosflux, yes a patch was deployed to WMF wikis)

Change 802632 had a related patch set uploaded (by Reedy; author: Majavah):

[mediawiki/core@master] SECURITY: ApiEditPage: update title after redirects

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

Change 802608 had a related patch set uploaded (by Reedy; author: Majavah):

[mediawiki/core@REL1_38] SECURITY: ApiEditPage: update title after redirects

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

Change 802608 merged by jenkins-bot:

[mediawiki/core@REL1_38] SECURITY: ApiEditPage: update title after redirects

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

Change 802632 merged by jenkins-bot:

[mediawiki/core@master] SECURITY: ApiEditPage: update title after redirects

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

Reedy added a subscriber: Reedy.

Merged to master and REL1_38 so it could be included in the 1.38.0 release; didn't see much reason holding it for 1.38.1 when it only affected releases without stable branches.

sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
sbassett added a subscriber: sbassett.

Time to mark this public and Resolved?

Yes.

sbassett changed Author Affiliation from N/A to Wikimedia Communities.Jun 3 2022, 9:25 PM
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed Risk Rating from N/A to Low.