If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Over-escaping forum-thread titles in the "Support Website Forums" page

VERIFIED FIXED in 1.3

Status

support.mozilla.org
General
--
critical
VERIFIED FIXED
8 years ago
8 years ago

People

(Reporter: stephend, Assigned: paulc)

Tracking

unspecified
x86
All
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: sumo_only, URL)

Attachments

(4 attachments)

(Reporter)

Description

8 years ago
(Not putting this into Forums, because I don't think this code is forum code, as that works properly in step 3.)

STR:

1. Load http://support.mozilla.com/en-US/kb/Support+Website+Forums
2. Look under "Most Popular Topics", at "I have the dreaded "Icanseek" a..."
3. Compare that to http://support.mozilla.com/tiki-view_forum_thread.php?locale=en-US&forumId=1&comments_parentId=386372

Expected Results:

"I have the dreaded "Icanseek" a..."

Actual Results:

See above
(Reporter)

Comment 1

8 years ago
Created attachment 389856 [details]
Screenshot

Bug is in the 5th forum title down from the left.
(Assignee)

Comment 2

8 years ago
"Support Website Forums" makes me sad.
Assignee: nobody → paul.craciunoiu
Target Milestone: --- → 1.2.1
(Assignee)

Comment 3

8 years ago
So I see this in the screenshot, but I created an article with the same title on my local copy, and it shows up alright. Stephen, could you maybe try to reproduce this on stage? That forum topic doesn't show on the page anymore on prod.

Comment 4

8 years ago
happens on stage too...
(Assignee)

Updated

8 years ago
Target Milestone: 1.2.1 → 1.3

Comment 5

8 years ago
Reopen if we can reproduce.
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → WORKSFORME
(Reporter)

Comment 6

8 years ago
OK, just reproduced.

https://support-stage.mozilla.org/en-US/kb/Support+Website+Forums
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---

Comment 7

8 years ago
Paul and I did investigation and so here's the conclusion:

We're escaping in the FORM that posts to the support forum (But not the forum that edits posts or posts to the contributor forum) so titles in the DATABASE are escaped (ugh).  We're then (likely) passing the raw title on tiki-view_forum and tiki-view_forum_thread without further escaping BUT the module that handles the forum front page IS doing a second escape.

The ideal situation is we DO NOT escape when entering new forum threads so the data in the table is the exact string entered and then we DO escape everywhere we display that text.  This would require making sure that we check everywhere we might be showing forum titles to make sure escaping is done.
(Assignee)

Comment 8

8 years ago
Created attachment 391291 [details] [diff] [review]
patch, v1

So as Cheng, Stephen and I discussed, there was some over-escaping going on -- in forum_advanced_post.php

It turns out that the escaping was done in php, which is risky and opens up vulnerabilities. Escaping should be done one display. This patch fixes that:
* comments out the php escaping in forum_advanced_post.php
* adds escaping on editing and viewing the forum topic titles in tiki-view_forum.php and tiki-view_forum_thread.php

I've looked carefully over these files so I hope I haven't missed anything. This will need some good QA to make sure we don't open up other vulnerabilities. However, strings were NOT previously properly escaped, as you can see here:
https://support-stage.mozilla.org/tiki-view_forum_thread.php?comments_parentId=374591&forumId=1
Notice <script> gets turned into <sc<x>ript> but <a> doesn't get escaped. This breaks, for example, here:
https://support-stage.mozilla.org/tiki-view_forum.php?locale=en-US&forumId=1 (see that the topic posted by Test contains 2 links in the title), potentially allowing spammers to replace the title link with their own -- see https://support-stage.mozilla.org/tiki-view_forum_thread.php?locale=en-US&comments_parentId=374592&forumId=1 whose title is broken professionally :)
Attachment #391291 - Flags: review?(laura)
(Assignee)

Updated

8 years ago
Severity: major → critical
Make sure you do not regress bug 500146.
(Assignee)

Comment 10

8 years ago
Thanks Reed. It doesn't seem to regress bug 500146 on URL:
http://support.mozilla.com/tiki-view_forum.php?locale=en-US&comments_offset=0&comments_threadId=0&comments_threshold=0&thread_sort_mode=lastPost_desc&forumId=1&forum_advanced_post=y&comment_topictype=n&comments_data=advanced%20post%20in%20use&plugins=%22%20style%3d%22background%3aurl%28javascript%3aalert%28%27XSS%20Test%20Successful%27%29%3b%29%22%20OA%3d%22&comments_title=123456&fx_version=123456&userfile1=123456&anonymous_name=123456&anonymous_email=123456&comments_postComment=Post&comments_previewComment=Preview&comments_postCancel=Cancel
(of course I tested locally)

At any rate, we should make sure to test this on stage too (hint hint QA).
(Assignee)

Updated

8 years ago
Blocks: 506101

Updated

8 years ago
Attachment #391291 - Flags: review?(laura) → review+
(Assignee)

Comment 11

8 years ago
r48845 / r48846
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED
(Reporter)

Comment 12

8 years ago
Created attachment 393552 [details]
Post-fix screenshot, forums
(Reporter)

Comment 13

8 years ago
Created attachment 393553 [details]
Post-fix screenshot, email notification
(Reporter)

Comment 14

8 years ago
Verified FIXED, w00t! :-)
Status: RESOLVED → VERIFIED
(Assignee)

Updated

8 years ago
Blocks: 511542

Updated

8 years ago
Whiteboard: sumo_only
You need to log in before you can comment on or make changes to this bug.