Closed Bug 526334 Opened 15 years ago Closed 14 years ago

Disallowed HTML is stripped instead of escaped

Categories

(addons.mozilla.org Graveyard :: Public Pages, defect, P4)

defect

Tracking

(Not tracked)

RESOLVED FIXED
4.x (triaged)

People

(Reporter: clouserw, Assigned: clouserw)

Details

(Whiteboard: [z])

Attachments

(3 files)

From bug 343573 comment 39:

The spec (in that bug) says "Any tags included that are not allowed should be displayed as entities rather than stripped out." 

This is just adding $this->config->set('Core.EscapeInvalidTags', true) however, we're passing the strings to our view sanitized, and then unsanitizing them when we need them in that form.  Our unsanitize() function uses preg_replace() which doesn't care how many replacements it does, meaning this is a regular occurance:  < -> < -> > .  This effectively ruins our sanitization with mixed levels of escaping.  The current code is stripping out the entities which is working fine.

We could fix this by passing the strings to the view individually with set() but that's pretty dirty.  I don't feel this is a high enough priority to block 5.3, so I'm splitting it into it's own lower priority bug.
Attached patch fixSplinter Review
This can be committed anytime
Assignee: nobody → clouserw
Target Milestone: --- → 5.5
Priority: -- → P4
r57822
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Wil: mind relaying a quick testcase?  Thanks!
Sure put this in a field that allows html: <script>alert('blah')</script>

On the live site it will disappear, on preview it will be shown on the page (escaped).
Had to back this patch out because it was making XSS possible.  Kicking out of 5.5. too.
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
Target Milestone: 5.5 → ---
Target Milestone: --- → 4.x (triaged)
Whiteboard: [z]
boom!
Status: REOPENED → RESOLVED
Closed: 15 years ago14 years ago
Resolution: --- → FIXED
Product: addons.mozilla.org → addons.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: