Page MenuHomePhabricator

Hovercards: XSS
Closed, ResolvedPublic

Description

There's a problem with the text-extracts, somehow related to <a> and <anchor> as seen in these 2 pages:

wrapping the tags in <tt> or <code> doesn't help fix it.

Attached:

Details

Reference
bz67180
Related Gerrit Patches:
mediawiki/extensions/Popups : REL1_24Run mw.html.escape on page extract and title
mediawiki/extensions/Popups : masterRun mw.html.escape on page extract and title
mediawiki/extensions/Popups : wmf/1.25wmf9Run mw.html.escape on page extract and title
mediawiki/extensions/Popups : wmf/1.25wmf10Run mw.html.escape on page extract and title
mediawiki/extensions/Popups : masterRun mw.html.escape on page extract and title

Event Timeline

bzimport raised the priority of this task from to Needs Triage.Nov 22 2014, 3:28 AM
bzimport added a project: Page-Previews.
bzimport set Reference to bz67180.
bzimport added a subscriber: Unknown Object (MLST).
Quiddity created this task.Jun 27 2014, 1:00 AM
Quiddity added a comment.EditedJun 27 2014, 1:30 AM

Description edited.

Quiddity updated the task description. (Show Details)Nov 25 2014, 1:10 AM
Quiddity added a project: TextExtracts.
Quiddity set Security to None.
Quiddity added a subscriber: MaxSem.
MaxSem renamed this task from Hovercards: TextExtract has anchor link?! to Hovercards: XSS.Nov 25 2014, 2:27 AM
MaxSem changed the visibility from "Public (No Login Required)" to "acl*security (Project)".
MaxSem changed Security from None to Software security bug.
Restricted Application changed the visibility from "acl*security (Project)" to "Custom Policy". · View Herald TranscriptNov 25 2014, 2:27 AM
Restricted Application changed the edit policy from "All Users" to "Custom Policy". · View Herald Transcript
Restricted Application added a project: acl*security. · View Herald Transcript

That's an XSS.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptNov 25 2014, 2:37 AM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
MaxSem added a subscriber: csteipp.Nov 25 2014, 6:18 PM
Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptNov 25 2014, 6:18 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

That's an XSS.

Confirmed. With the extension enabled, you can link to [[User:Stype & Co. (WMF)/scripts]] for a PoC.

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptNov 25 2014, 6:25 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

As pointed out by Max on irc, it looks like this is due to https://gerrit.wikimedia.org/r/#/c/132707/

@Prtksxna, this is why I specifically asked you to put a comment by that code-- https://phabricator.wikimedia.org/T63743#anchor-665654, can you guys get this fixed asap?

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptNov 25 2014, 9:31 PM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript

Patch at https://gerrit.wikimedia.org/r/175933

(Note that @Prtksxna cannot access this bug, currently)

Restricted Application changed the visibility from "Custom Policy" to "Custom Policy". · View Herald TranscriptNov 26 2014, 7:16 AM
Restricted Application changed the edit policy from "Custom Policy" to "Custom Policy". · View Herald Transcript
csteipp changed the visibility from "Custom Policy" to "Custom Policy".Nov 26 2014, 2:45 PM
csteipp changed the edit policy from "Custom Policy" to "Custom Policy".
csteipp changed Security from Software security bug to None.

I don't think that's going to fix it, but I'm getting it setup in dev.

(Note that @Prtksxna cannot access this bug, currently)

Should have access now

Proposed patch.

@Prtksxna, in general, we don't put security fixes in gerrit until we have the cluster patched.

We'll deploy that fix, and then work on a better solution-- in general, .html() should rarely be used and I'd rather see this fixed with something like (after you remove the leading instance of the page title from page.extract),

$contentbox.append( $('<b></b>').text( page.title ) );
$contentbox.append( $('<span></span>').text( page.extract ) );

MaxSem closed this task as Resolved.Dec 1 2014, 7:45 PM
MaxSem claimed this task.

Fix deployed.

MaxSem changed the visibility from "Custom Policy" to "Public (No Login Required)".
Prtksxna moved this task from Backlog to Archive on the Page-Previews board.Dec 3 2014, 2:59 AM

Change 180434 had a related patch set uploaded (by Mglaser):
Run mw.html.escape on page extract and title

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

Patch-For-Review

Change 180434 merged by jenkins-bot:
Run mw.html.escape on page extract and title

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