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 "Security (Project)".
MaxSem changed Security from None to Software security bug.
Restricted Application changed the visibility from "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: 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