Page MenuHomePhabricator

Wikidata description on mobile search and Special:Nearby is not escaped
Closed, ResolvedPublic

Description

If i have javascript in a wikibase description (e.g. <script>alert( 'hi' );</script>) and then search for it (or the item is nearby) then the javascript is injected instead of being escaped.

Event Timeline

Maniphest changed the visibility from "Public (No Login Required)" to "Custom Policy".Feb 3 2016, 6:03 PM
Maniphest changed the edit policy from "All Users" to "Custom Policy".
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 3 2016, 6:03 PM
aude created this task.Feb 3 2016, 6:03 PM
aude updated the task description. (Show Details)
aude added projects: MobileFrontend, Security.
aude changed Security from None to Software security bug.
aude edited subscribers, added: aude, Jdlrobson, daniel; removed: Aklapper.
bd808 triaged this task as Unbreak Now! priority.Feb 3 2016, 6:08 PM
bd808 added a subscriber: bd808.
bd808 added a subscriber: bmansurov.
bd808 added a comment.Feb 3 2016, 6:16 PM

Reminder, the patch for this should not go into gerrit if this is a true XSS. It should be added as an attachment here.

aude added a comment.Feb 3 2016, 6:21 PM

to be clear, this happens in Special:Nearby and mobile search. (and probably watchlist) afaik, there are different/multiple code paths for these.

daniel added a subscriber: csteipp.Feb 3 2016, 6:43 PM
Jdlrobson claimed this task.

Wow. Seems that we just treat it as HTML and it's not clear why. Can Wikidata descriptions contain safe HTML e.g. <strong> tags?

Can confirm this is on the cluster o_O
Luckily fix is easy:

Let me make a real patch..

should be SWATTed.

Patch is deployed,

19:09 csteipp: deployed patch for T125684

Krenair added a subscriber: Krenair.Feb 3 2016, 7:17 PM

should be SWATTed.

Security patches don't go out in SWAT windows, they're not publicly scheduled.

csteipp closed this task as Resolved.Feb 3 2016, 7:17 PM
daniel added a comment.Feb 3 2016, 7:19 PM

Wow. Seems that we just treat it as HTML and it's not clear why. Can Wikidata descriptions contain safe HTML e.g. <strong> tags?

No. Wikibase terms (labels, descriptions, and aliases) are plain text. They need escaping before they can be used in HTML (or even in wikitext, though that would just get confusing, not dangerous).

daniel added a comment.Feb 3 2016, 7:21 PM

@Jdlrobson: thanks for the quick response!

daniel added a comment.Feb 3 2016, 7:22 PM

@Krenair: so is the fix in master now? Or should it now go to gerrit?

It will have to go to master via gerrit. If this was code in a release branch it will have to be in the next MW security release before being put into gerrit

And indeed Chris marked this as a release blocker

Jdlrobson reopened this task as Open.Feb 3 2016, 9:40 PM

The patch isn't in current MobileFrontend master. @csteipp is it safe to post it publically now?

csteipp added a subscriber: demon.Feb 3 2016, 10:20 PM

I'm not sure how we're handling mobilefrontend-- @demon, do we want to wait for the next release, or should they just push this now?

demon added a comment.Feb 4 2016, 3:41 PM

I'm not sure how we're handling mobilefrontend-- @demon, do we want to wait for the next release, or should they just push this now?

It's not bundled, so I'd just push and announce it publicly for those using it.

It's not bundled, so I'd just push and announce it publicly for those using it.

Cool. So @Jdlrobson, merge in gerrit, close and make this task public, and send a message about it to wikitech-l (and maybe mediawiki-l). Can you or someone on your team handle that?

Jdlrobson changed Security from Software security bug to None.Feb 4 2016, 5:58 PM
Jdlrobson changed Security from None to Software security bug.
Jdlrobson changed Security from Software security bug to None.Feb 4 2016, 6:55 PM

Email sent to wikitech-l and mobile-l - "MobileFrontend+Wikibase security patch"
@phuedx could you sign off?

Has to be public before @phuedx can see it.

Krenair closed this task as Resolved.Feb 4 2016, 6:59 PM
Krenair changed the visibility from "Custom Policy" to "Public (No Login Required)".
Krenair changed the edit policy from "Custom Policy" to "All Users".
Restricted Application added a subscriber: Luke081515. · View Herald TranscriptFeb 4 2016, 6:59 PM
Florian added a subscriber: Florian.Feb 4 2016, 7:39 PM
phuedx added a subscriber: phuedx.Feb 11 2016, 6:11 AM

I've verified that neither search nor the watchlist are affected on the Beta Cluster. I updated Q77282 and tested both features. However, it's hard to verify Special:Nearby as it's hitting the enwiki API rather than the local API.