XSS in Flow topic titles
Closed, ResolvedPublic

Description

Split from T120291#1850816:

I wasn't able to trigger an XSS in the Echo flyout, but I was in Flow...

Simply put <script>alert(1)</script> in a topic title.

Catrope created this task.Dec 3 2015, 11:58 PM
Catrope updated the task description. (Show Details)
Catrope raised the priority of this task from to Unbreak Now!.
Catrope added subscribers: Catrope, Legoktm.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptDec 3 2015, 11:58 PM

Change 256858 had a related patch set uploaded (by Catrope):
[SECURITY] Escape HTML characters in topic titles

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

Change 256867 had a related patch set uploaded (by CSteipp):
Add warning comment on formatLinksInComment

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

Change 256858 merged by jenkins-bot:
[SECURITY] Escape HTML characters in topic titles

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

Change 256867 merged by jenkins-bot:
Add warning comment on formatLinksInComment

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

Legoktm added a subscriber: csteipp.Dec 4 2015, 2:10 AM

I audited the other callers of Linker::formatLinksInComment() (CentralAuth and LQT) and they both call the Sanitizer first properly. I'll leave it to either @Catrope or @csteipp to mark this as resolved.

csteipp closed this task as Resolved.Dec 4 2015, 5:13 PM

The issue is resolved, so I'm going to close this.

SBisson, if there are ways I can support the Collab team's QA process to make sure we don't have this happen again, let me know. Let's make that a followup phab task, if there's anything we can identify.

Etonkovidova added a subscriber: Etonkovidova.

Checked in betalabs - title, reply, board description, summary look fine.

There is the same issue with displaying html in a topic title as in T119537: Links in Flow topic titles: html code displayed when page scrolls up

SBisson added a subscriber: SBisson.Dec 4 2015, 5:22 PM

SBisson, if there are ways I can support the Collab team's QA process to make sure we don't have this happen again, let me know. Let's make that a followup phab task, if there's anything we can identify.

It's a good question. @Etonkovidova, our QA, might have some ideas.

Please review the following - any suggestions(follow-up bugs/more testing) are welcomed.

Tested on test.wikipedia.org

dataFlow topic titlesEcho flyoutNotification page
<code></code>plain textas code textas code text
<script>alert(1)</script>plain textplain textplain text
<tt>plain textis not displayedmonospace; changes all text to monospace
{{release}}plain textred linkred link
<!--T:1-->plain textplain textplain text
<translate>plain textplain textplain text

Red links for templates and <code> text displayed on Notifications page and in Echo.


Monospace in Echo notifications

<tt> in a topic titles displayed on Notifications page changed all subsequent notification messages font to monospace (and, in fact, all page's fonts)

Restricted Application added a subscriber: Luke081515. · View Herald TranscriptJan 22 2016, 4:16 PM
This comment was removed by Etonkovidova.

Please review the following - any suggestions(follow-up bugs/more testing) are welcomed.

Tested on test.wikipedia.org

Thanks for testing. The *only* markup topic titles support are internal links ([[Earth]]) and media links ([[Media:Saturn.jpg]]).

That means all cells of that table should be plain text.

The red links for templates is only because the template didn't exist. If it did, it would render, which is also wrong (templates are not supported).

Some of this is T120291: wikitext in flow titles is parsed (HTML tags like <tt> and templates) on echo notifications.

@Mattflaschen To clarify, did you re-open due to further security issues, or do you just have related concerns with title handling that do not have security implications.

If its for follow-up things that aren't security, could that be done in a separate bug?

Mattflaschen-WMF closed this task as Resolved.Jan 26 2016, 11:02 PM

I don't think any of the remaining parts are strictly security.

I'll use T120291: wikitext in flow titles is parsed (HTML tags like <tt> and templates) on echo notifications and file a new bug if I find Flow itself displaying it wrong anywhere.

Restricted Application added a subscriber: Malyacko. · View Herald TranscriptMar 3 2016, 7:07 PM