Page MenuHomePhabricator

XSS bug in Twinkle friendlytalkback gadget across many wikis
Closed, ResolvedPublic

Description

An enwiki Twinkle maintainer (Amorymeltzer) has PM'd me with a XSS problem, it's apparently just a .append -> .text change. Twinkle bugs aren't tracked in Phab, but I'm opening this here to give appropriate people visibility into the problem existing - I'll take care of rolling the fix out across the wikis.

Event Timeline

Krenair created this task.Jan 5 2020, 10:08 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJan 5 2020, 10:08 PM
Krenair updated the task description. (Show Details)Jan 5 2020, 10:09 PM
Krenair updated the task description. (Show Details)
Krenair added a subscriber: Amorymeltzer.
Krenair renamed this task from XSS bug in Twinkle gadget across many wikis to XSS bug in Twinkle friendlytalkback gadget across many wikis.Jan 5 2020, 10:17 PM

Mostly rolled out the fix - I don't have the rights to do it on foundationwiki (it's not part of SUL so my GIE flag doesn't apply there), and zhwiki is not letting me save for some reason

Krenair added a comment.EditedJan 5 2020, 10:43 PM

Done zhwiki - I just had to edit the edit page DOM to remove the disabled flag on the save button. Not sure what that was about.
Edit: Turns out it's because that wiki tries to enforce that you preview before saving.

Krenair added a comment.EditedJan 5 2020, 10:50 PM

The only remaining wiki (based on https://tools.wmflabs.org/global-search/?namespaces=8&q=append%5C%28Twinkle%5C.talkback%5C.optout%5C%29&regex=1) is foundationwiki - @Varnent please take care of https://foundation.wikimedia.org/wiki/MediaWiki:Gadget-friendlytalkback.js by changing both instances of .append(Twinkle.talkback.optout); to .text(Twinkle.talkback.optout); (as was done on many other wikis) as you seem to be the only person in Wikimedia who holds editsitejs on foundationwiki right now (I think we should fix that at some point). Alternatively we can find a steward to grant someone else interface admin there, or maybe get the page deleted (why does foundationwiki even have Twinkle, it's a fishbowl?)

sbassett triaged this task as High priority.Jan 6 2020, 4:26 PM
sbassett moved this task from Backlog / Other to External (Non-WMF) Issues on the acl*security board.
sbassett added a project: Security-Team.
sbassett moved this task from Incoming to Watching on the Security-Team board.

@sbassett: "External (Non-WMF) Issues" does not really feel correct here? This is about WMF websites...

@Aklapper - the Security-Team would classify most gadget maintenance issues not related to the extension or configuration code itself (including security-related issues) as on-wiki issues, typically handled by sysops and the like, external to the wmf (even if some happen to be wmf employees). The "External (Non-WMF) Issues" workboard column seemed the most appropriate given this reasoning, though we could potentially rename it or even add a new, more specifically-named column.

I've granted myself i-admin and made the change.

Thanks James.
@Amorymeltzer: I think the fix for this is fully rolled out now, can you confirm?
If confirmed please can someone make this task public.

All clear AFAICT, thanks all for the swift work.

sbassett closed this task as Resolved.Jan 7 2020, 8:53 PM
sbassett claimed this task.
sbassett reassigned this task from sbassett to Krenair.
sbassett moved this task from External (Non-WMF) Issues to Done on the acl*security board.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".

Sorry to bug you again @Krenair but at your leisure, could I bother you for a tidy-up I've caused? There's no security issue thanks to this, but my change meant there's now an omnipresent "false" message that shouldn't be there. If you could do another simple change on those same Twinkle pages (listed here in global search) I'd be quite grateful. See https://github.com/azatoth/twinkle/pull/797 and https://github.com/azatoth/twinkle/pull/798

Change: if ($status.length)
To: if ($status.length && Twinkle.talkback.optout)

Sorry to bug you again @Krenair but at your leisure, could I bother you for a tidy-up I've caused? There's no security issue thanks to this, but my change meant there's now an omnipresent "false" message that shouldn't be there. If you could do another simple change on those same Twinkle pages (listed here in global search) I'd be quite grateful. See https://github.com/azatoth/twinkle/pull/797 and https://github.com/azatoth/twinkle/pull/798

Change: if ($status.length)
To: if ($status.length && Twinkle.talkback.optout)

done - again with the exception of foundationwiki