Special:MergeHistory doesn't verify the edit token (it's vulnerable to CSRF). This is a regression from rMW598068334e72: Convert Special:MergeHistory to use OOUI. (https://gerrit.wikimedia.org/r/#/c/287435/). The simplest fix is to revert that patch. Somebody should also check Sethakill's other commits, since he did several similar OOUI conversions.
Description
Description
Details
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Revert "Convert Special:MergeHistory to use OOUI." | mediawiki/core | master | +186 -117 |
Related Objects
Related Objects
- Mentioned In
- T134675: Convert Special:MergeHistory to OOUI
- Mentioned Here
- rMW598068334e72: Convert Special:MergeHistory to use OOUI.
Event Timeline
Comment Actions
(I'll note that merging page histories can be very disruptive and is difficult to undo.)
Comment Actions
There doesn't seem to be a fix deployed still (or at least not a revert, I didn't try testing this in production), so perhaps I should just submit a public revert and have it SWAT-ted on Monday?
Comment Actions
Somebody should also check Sethakill's other commits, since he did several similar OOUI conversions.
The only other write special pages they converted were Unlockdb/Lockdb which was already using HTMLForm and looks fine.
Comment Actions
There are also some code quality issues in the new Special:MergeHistory so overall a revert seems sensible.
Comment Actions
Deployed to wmf.8 by @thcipriani. Could anyone from Security confirm that it is okay to make the task public, commit the patch to master and close the task? (@dpatrick @Bawolff?)
Comment Actions
If it's being reverted then can't we just revert on master and avoid having to maintain a messy security patch on each new branch?
The patch did not apply cleanly to wmf.9, though I was able to fix the conflict. Still it would be much more straightforward to just revert on master.
Comment Actions
When it's a security bug that affects WMF wikis, we want the wikis fixed before we make the bug public by putting the patch in Gerrit. In this case, since the bug wasn't in a released version (1.27 or earlier) we can probably make it public now instead of having to wait for the next security release, but we usually wait for the Security team to make the final decision.
Comment Actions
Reverting such a large change is probably pretty noticeable. I might argue that the presence of this patch on live wikis counts as a public disclosure ;)
I'm not intending to dispute the process though. Hopefully Security will respond before this patch lives on for another week.
Comment Actions
It's fine to go ahead and make this public and commit to Gerrit. Thanks for your work on this everyone.
Comment Actions
I wanted to have the patch made public and committed to master immediately after deploying it, but @thcipriani and I were not comfortable making a security patch public by ourselves, and no one from the security team was available (and no one responded to my question here in T138346#2404165). @mmodell the first patch I posted here applies cleanly to wmf.9 and master (T138346#2428732).
For the future, @dpatrick, what do I need to do to have security issues like this resolved more quickly, before people merge conflicting changes to the same code? It's kind of sad that no one picked this up for two weeks until I scheduled a SWAT deployment for it…
Comment Actions
Change 297544 had a related patch set uploaded (by Bartosz Dziewoński):
Revert "Convert Special:MergeHistory to use OOUI."
Comment Actions
Change 297544 merged by jenkins-bot:
Revert "Convert Special:MergeHistory to use OOUI."
Comment Actions
@matmarex, you can feel free to contact me on IRC, Google Hangouts/GChat, or direct e-mail (rather than a Phab ticket; I get a lot of Phab e-mail, filter it, and look at it less frequently throughout the day than I do non-Phab messages). As for two weeks passing from the opening of the bug until its resolution, unfortunately, this happened during a busy period of time, with scheduled travel for Wikimania, scheduled time off, and reduced capacity on the Security team. I apologize if this put you (all) in a tight spot/created more work for you.
Comment Actions
Thanks, I see. To be honest, it couldn't have taken more than an hour or two of my time (and I guess another hour for the deployers). Mostly I'm worried that there didn't seem to be anyone taking responsibility for this. But it's true that there were many things happening at the same time.