Page MenuHomePhabricator

XSS vulnerability in CologneBlue
Open, LowPublicSecurity

Description

this was flagged by @Daimona on the review of https://gerrit.wikimedia.org/r/c/mediawiki/skins/CologneBlue/+/637771/2/includes/CologneBlueTemplate.php#395

Set the qbfind message to:

"<script>alert(0);</script>Findz",`

Cologne blue will alert 0.

This happens in current master and was only flagged(noticing) during adjustments to the code.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptWed, Nov 4, 10:57 PM

Urgh, I'm sorry. I didn't realize master was already affected.

Jdlrobson updated the task description. (Show Details)Wed, Nov 4, 11:13 PM
Jdlrobson added a subscriber: Ammarpad.
Urbanecm triaged this task as High priority.Thu, Nov 5, 1:16 AM
Urbanecm added projects: Vuln-XSS, CologneBlue.
Urbanecm added subscribers: matmarex, DannyS712.
Urbanecm added a subscriber: Urbanecm.

I don't think this needs to be a security task. This is a system message, and so it's only vulnerable to administrators. We have a public tracking task for issues like this (T2212). We should just submit a patch to Gerrit and merge it.

Let me know if I shouldn't just submit the patch to Gerrit. Otherwise I'll do it tomorrow.

I don't think this needs to be a security task. This is a system message, and so it's only vulnerable to administrators. We have a public tracking task for issues like this (T2212). We should just submit a patch to Gerrit and merge it.

That's true, although I can remember of some similar tasks being kept sec-protected and assigned a CVE recently. Just because exploiting is presumably hard (but who knows, perhaps there's a workaround to being administrators), it's still a possible attack vector: you only need to compromise a sysop account, which is likely much easier than compromising an IA account. So I'd be paranoid and keep it protected. As for the fix, I guess it can be pushed on gerrit as long as someone is around to sec-deploy it.

Either way, I'm obviously leaving this up to the SecTeam (and approving your patch, just in case).

sbassett added a subscriber: sbassett.EditedThu, Nov 5, 6:38 PM

Let me know if I shouldn't just submit the patch to Gerrit. Otherwise I'll do it tomorrow.

+1 to the patch above and this is low risk to go to gerrit IMO, as the issue was already publicly-flagged in gerrit and I'm not seeing anything remotely suspicious within any of the existing messages after a quick audit.

Great. I'm happy to merge this Bartosz if you want to throw it up onto Gerrit!

sbassett added a comment.EditedThu, Nov 5, 6:43 PM

That's true, although I can remember of some similar tasks being kept sec-protected and assigned a CVE recently.

Yes, I'll likely track this for the upcoming supplemental security announcement (T263810), though since the skin is no longer bundled, once the patch is merged and deployed, this task can be made public and then any relevant backports can be completed. This probably does warrant a CVE as it's still a viable XSS under certain circumstances, so I'll likely eventually request one.

Just because exploiting is presumably hard (but who knows, perhaps there's a workaround to being administrators), it's still a possible attack vector: you only need to compromise a sysop account, which is likely much easier than compromising an IA account. So I'd be paranoid and keep it protected. As for the fix, I guess it can be pushed on gerrit as long as someone is around to sec-deploy it.

Yes, agreed. I'm hopeful the merge/deploy can happen quickly. If not, I'm happy to sec-deploy it once the patch is merged.

Ok, the patch is up for review (@Jdlrobson et al):

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

If we could get this reviewed and merged soon, that'd be great.

sbassett lowered the priority of this task from High to Low.Mon, Nov 9, 4:35 PM
sbassett moved this task from Incoming to Watching on the Security-Team board.
sbassett added a subscriber: Reedy.

This fix was deployed to production (1, 2, tx @Reedy!). I'm tracking this task for the next supplemental release: T263810. So we can likely make this task public soon and work on any relevant backports and requesting the CVE.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Mon, Nov 9, 10:29 PM

Change 640194 had a related patch set uploaded (by SBassett; owner: Bartosz Dziewoński):
[mediawiki/skins/CologneBlue@REL1_34] SECURITY: Fix escaping of the 'qbfind' message

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

Change 640195 had a related patch set uploaded (by SBassett; owner: Bartosz Dziewoński):
[mediawiki/skins/CologneBlue@REL1_31] SECURITY: Fix escaping of the 'qbfind' message

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

Change 640194 merged by jenkins-bot:
[mediawiki/skins/CologneBlue@REL1_34] SECURITY: Fix escaping of the 'qbfind' message

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

Change 640195 merged by SBassett:
[mediawiki/skins/CologneBlue@REL1_31] SECURITY: Fix escaping of the 'qbfind' message

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