Page MenuHomePhabricator
Create Task
Maniphest T286385

XSS in GlobalWatchlist (CVE-2021-42046)
Closed, ResolvedPublic2 Estimated Story PointsSecurity
Actions

Description

As I was reviewing existing code in a different context, I realized that we don't escape the uses of mw.msg in watchlistUtils.makeUserLinks, which returns raw html. This can create an XSS if the messages rev-deleted-user or ntimes are manipulated to try and include scripts.

The solution is to add mw.html.escape() calls or use mw.Message.escaped()
Given that this hasn't (as far as I can tell) been noticed anywhere, and there will be a bunch of unrelated changes to the relevant file that may cause merge conflicts if the fix is deployed as a normal security patch, would it be okay to send this publicly on gerrit? Commit message would be along the lines of "Clean up watchlistUtils.makeUserLinks" and I would replace the mw.msg() shortcut with creation of mw.Message objects and calling .escaped().

Relevant code is at https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/extensions/GlobalWatchlist/+/141326cdcce048b2b764fdd00181c123463572ab/modules/watchlistUtils.js#84

You can confirm by setting the message rev-deleted-user to (username removed)</span><script>alert('error')</script><span>

Details

Risk Rating
Low
Author Affiliation
Wikimedia Communities
ProjectBranchLines +/-Subject
mediawiki/extensions/GlobalWatchlistREL1_36+2 -2Fix creation of mw.Message objects
mediawiki/extensions/GlobalWatchlistREL1_36+5 -2Clean up to watchlistUtils.makeUserLinks
mediawiki/extensions/GlobalWatchlistwmf/1.37.0-wmf.14+2 -2Fix creation of mw.Message objects
mediawiki/extensions/GlobalWatchlistwmf/1.37.0-wmf.14+5 -2Clean up to watchlistUtils.makeUserLinks
mediawiki/extensions/GlobalWatchlistmaster+2 -2Fix creation of mw.Message objects
mediawiki/extensions/GlobalWatchlistmaster+5 -2Clean up to watchlistUtils.makeUserLinks
Customize query in gerrit

Related Objects
Search...

Event Timeline

DannyS712 created this task.Jul 10 2021, 12:32 AM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptJul 10 2021, 12:32 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 added projects: Vuln-XSS, MediaWiki-extensions-GlobalWatchlist.Jul 10 2021, 12:33 AM
DannyS712 moved this task from Unsorted to Next on the User-DannyS712 board.
DannyS712 added subscribers: Legoktm, ashley.
sbassett moved this task from Incoming to Our Part Is Done on the Security-Team board.Jul 12 2021, 2:44 PM
sbassett added a project: SecTeam-Processed.
sbassett added a subscriber: sbassett.

... would it be okay to send this publicly on gerrit? Commit message would be along the lines of "Clean up watchlistUtils.makeUserLinks"

Yes, issues like this are generally fine to push through gerrit, since several similar issues have been fixed publicly under T2212. If you'd like, we can keep this bug private for now (even though the patch would obviously disclose the issue) and maybe deploy this sooner via a backport window if it misses this week's train.

DannyS712 added a subscriber: gerritbot.Jul 12 2021, 4:40 PM
gerritbot added a comment.Jul 12 2021, 4:40 PM

Change 703903 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Clean up to watchlistUtils.makeUserLinks

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

gerritbot added a project: Patch-For-Review.Jul 12 2021, 4:40 PM
gerritbot added a comment.Jul 13 2021, 10:28 PM

Change 703903 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Clean up to watchlistUtils.makeUserLinks

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

jenkins-bot mentioned this in rEGLW5817ddb42ad9: Clean up to watchlistUtils.makeUserLinks.Jul 13 2021, 10:28 PM
DannyS712 added a comment.Jul 13 2021, 10:31 PM
In T286385#7205743, @sbassett wrote:

... would it be okay to send this publicly on gerrit? Commit message would be along the lines of "Clean up watchlistUtils.makeUserLinks"

Yes, issues like this are generally fine to push through gerrit, since several similar issues have been fixed publicly under T2212. If you'd like, we can keep this bug private for now (even though the patch would obviously disclose the issue) and maybe deploy this sooner via a backport window if it misses this week's train.

It missed this week's train, do we want to backport it?

Also, should this be backported to 1.36? I would say no - we should only maintain the MASTER branch of GlobalWatchlist and not any of the prior releases because its mostly intended to be used by WMF wikis

DannyS712 moved this task from Backlog to In progress on the MediaWiki-extensions-GlobalWatchlist board.Jul 14 2021, 1:08 AM
gerritbot added a comment.Jul 14 2021, 1:12 AM

Change 704378 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@master] Fix creation of mw.Message objects

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

gerritbot added a comment.Jul 14 2021, 2:08 AM

Change 704378 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@master] Fix creation of mw.Message objects

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

DannyS712 mentioned this in rEGLW44bcd0177acb: Fix creation of mw.Message objects.Jul 14 2021, 2:08 AM
Jdforrester-WMF added a project: MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 14 2021, 10:05 AM
sbassett added a comment.Jul 14 2021, 4:43 PM
In T286385#7210901, @DannyS712 wrote:

It missed this week's train, do we want to backport it?

Can we try to get it scheduled for tomorrow's backport window?

Also, should this be backported to 1.36? I would say no - we should only maintain the MASTER branch of GlobalWatchlist and not any of the prior releases because its mostly intended to be used by WMF wikis

As the official maintainer of the extension, I suppose the final call is up to you. I would note that the Security-Team generally prefers to backport security-related issues to any relevant release branches regardless of their overall/actual usage outside of Wikimedia production.

DannyS712 added a comment.Jul 14 2021, 8:22 PM
In T286385#7212835, @sbassett wrote:
In T286385#7210901, @DannyS712 wrote:

It missed this week's train, do we want to backport it?

Can we try to get it scheduled for tomorrow's backport window?

I should be able to be around. Note that there are two patches - the first accidentally broke things and the second fixed it properly. They would need to be deployed together.

Also, should this be backported to 1.36? I would say no - we should only maintain the MASTER branch of GlobalWatchlist and not any of the prior releases because its mostly intended to be used by WMF wikis

As the official maintainer of the extension, I suppose the final call is up to you. I would note that the Security-Team generally prefers to backport security-related issues to any relevant release branches regardless of their overall/actual usage outside of Wikimedia production.

In that case, I have no objection to back porting them both to 1.36 (there is no 1.35 branch or earlier). Should we wait until closer to the security release of 1.36.2 to avoid calling attention to the fact that this was a security issue?

DannyS712 removed a project: Patch-For-Review.Jul 14 2021, 8:23 PM
sbassett added a comment.EditedJul 14 2021, 8:29 PM
In T286385#7213384, @DannyS712 wrote:

I should be able to be around. Note that there are two patches - the first accidentally broke things and the second fixed it properly. They would need to be deployed together.

Should be fine, just need to get them added to the schedule, I believe. If that doesn't happen, I can sec-deploy them, but I'd rather not if we can get them pushed during a backport window.

In that case, I have no objection to back porting them both to 1.36 (there is no 1.35 branch or earlier). Should we wait until closer to the security release of 1.36.2 to avoid calling attention to the fact that this was a security issue?

We can. Since GlobalWatchlist isn't bundled and the issue is already quasi-disclosed via gerrit, I'll track it on the next supplemental announcement task: T285414

sbassett mentioned this in T285414: Write and send supplementary release announcement for extensions and skins with security patches (1.31.16/1.35.4/1.36.2).Jul 14 2021, 8:31 PM
DannyS712 added a comment.Jul 15 2021, 3:54 PM

You can test this in your browser to confirm the security fix without changing the sitewide mediawiki messages by, if you have an entry with the user revision deleted, running mw.messages.set( 'rev-deleted-user', "(username removed)</span><script>alert('error')</script><span>" ); in the console and then refreshing

gerritbot added a comment.Jul 15 2021, 11:06 PM

Change 704814 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@wmf/1.37.0-wmf.14] Clean up to watchlistUtils.makeUserLinks

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

gerritbot added a project: Patch-For-Review.Jul 15 2021, 11:06 PM

Change 704815 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@wmf/1.37.0-wmf.14] Fix creation of mw.Message objects

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

gerritbot added a comment.Jul 15 2021, 11:14 PM

Change 704814 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@wmf/1.37.0-wmf.14] Clean up to watchlistUtils.makeUserLinks

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

DannyS712 mentioned this in rEGLW691c91616d19: Clean up to watchlistUtils.makeUserLinks.Jul 15 2021, 11:14 PM
DannyS712 added a subscriber: Stashbot.Jul 15 2021, 11:16 PM
DannyS712 edited subscribers, added: Stashbot; removed: Stashbot.
gerritbot added a comment.Jul 15 2021, 11:17 PM

Change 704815 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@wmf/1.37.0-wmf.14] Fix creation of mw.Message objects

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

DannyS712 mentioned this in rEGLW126cad939ee9: Fix creation of mw.Message objects.Jul 15 2021, 11:17 PM
Stashbot added a comment.Jul 15 2021, 11:28 PM

Mentioned in SAL (#wikimedia-operations) [2021-07-15T23:28:53Z] <brennen@deploy1002> Synchronized php-1.37.0-wmf.14/extensions/GlobalWatchlist/modules/watchlistUtils.js: Backport: [[gerrit:704815|Fix creation of mw.Message objects (T286385)]] (duration: 00m 57s)

DannyS712 claimed this task.Jul 15 2021, 11:38 PM
DannyS712 moved this task from In progress to Done on the MediaWiki-extensions-GlobalWatchlist board.
DannyS712 changed Author Affiliation from N/A to Wikimedia Communities.
DannyS712 set the point value for this task to 2.
DannyS712 moved this task from Next to In progress on the User-DannyS712 board.

Okay, fixed in master and backported to current wmf branch (1.37.0-wmf.14), needs backport to 1.36 as part of T285414: Write and send supplementary release announcement for extensions and skins with security patches (1.31.16/1.35.4/1.36.2) but other than that this should be done, right? Should we keep it open until the 1.36 backport? Or is it okay to close as resolved?

DannyS712 removed a project: Patch-For-Review.Jul 15 2021, 11:39 PM
Jdforrester-WMF edited projects, added MW-1.37-notes (1.37.0-wmf.14; 2021-07-12); removed MW-1.37-notes (1.37.0-wmf.15; 2021-07-19).Jul 16 2021, 9:46 AM
sbassett triaged this task as Low priority.EditedJul 20 2021, 2:06 AM
sbassett moved this task from Our Part Is Done to Watching on the Security-Team board.
In T286385#7216422, @DannyS712 wrote:

... but other than that this should be done, right? Should we keep it open until the 1.36 backport? Or is it okay to close as resolved?

  1. Yes, save the 1.36 backport and disclosure via the supplemental security announcement.
  2. I think we usually do this so as to avoid confusion as to whether the issue is completely resolved.
  3. If you really want to resolve it, we can, but I'd rather keep this open to avoid confusion. You can certainly resolve it on your user board.
DannyS712 added a comment.Jul 20 2021, 2:22 AM
In T286385#7223222, @sbassett wrote:
In T286385#7216422, @DannyS712 wrote:

... but other than that this should be done, right? Should we keep it open until the 1.36 backport? Or is it okay to close as resolved?

  1. Yes, save the 1.36 backport and disclosure via the supplemental security announcement.
  2. I think we usually do this so as to avoid confusion as to whether the issue is completely resolved.
  3. If you really want to resolve it, we can, but I'd rather keep this open to avoid confusion. You can certainly resolve it on your user board.

Thanks for explaining, will leave open

sbassett added a parent task: T285414: Write and send supplementary release announcement for extensions and skins with security patches (1.31.16/1.35.4/1.36.2).Sep 10 2021, 3:06 AM
gerritbot added a comment.Oct 1 2021, 11:26 PM

Change 725282 had a related patch set uploaded (by Mstyles; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@REL1_36] Clean up to watchlistUtils.makeUserLinks

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

gerritbot added a project: Patch-For-Review.Oct 1 2021, 11:26 PM

Change 725282 abandoned by Mstyles:

[mediawiki/extensions/GlobalWatchlist@REL1_36] Clean up to watchlistUtils.makeUserLinks

Reason:

wrong order

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

gerritbot added a comment.Oct 4 2021, 7:03 PM

Change 725282 restored by SBassett:

[mediawiki/extensions/GlobalWatchlist@REL1_36] Clean up to watchlistUtils.makeUserLinks

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

gerritbot added a comment.Oct 4 2021, 7:11 PM

Change 725282 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@REL1_36] Clean up to watchlistUtils.makeUserLinks

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

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Oct 4 2021, 7:15 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
sbassett removed a project: Patch-For-Review.
sbassett closed this task as Resolved.Oct 4 2021, 9:12 PM
DannyS712 reopened this task as Open.Oct 4 2021, 10:42 PM

rEGLW44bcd0177acb: Fix creation of mw.Message objects also needs to be backported

gerritbot added a comment.Oct 4 2021, 10:42 PM

Change 725920 had a related patch set uploaded (by DannyS712; author: DannyS712):

[mediawiki/extensions/GlobalWatchlist@REL1_36] Fix creation of mw.Message objects

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

gerritbot added a project: Patch-For-Review.Oct 4 2021, 10:42 PM
gerritbot added a comment.Oct 5 2021, 2:37 PM

Change 725920 merged by jenkins-bot:

[mediawiki/extensions/GlobalWatchlist@REL1_36] Fix creation of mw.Message objects

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

sbassett added a comment.Oct 5 2021, 2:42 PM
In T286385#7400578, @DannyS712 wrote:

rEGLW44bcd0177acb: Fix creation of mw.Message objects also needs to be backported

Ok, now merged, thanks. And also tracked for the supplemental (T285414), which will be released within a couple of days.

DannyS712 closed this task as Resolved.Oct 5 2021, 3:47 PM

Moving back to resolved since the backport was merged

Mstyles renamed this task from XSS in GlobalWatchlist to XSS in GlobalWatchlist (CVE-2021-42046).Oct 7 2021, 8:33 PM
Content licensed under Creative Commons Attribution-ShareAlike 4.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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