Closed
Bug 901894
Opened 11 years ago
Closed 7 years ago
Centralize HTML sanitization Code
Categories
(Firefox OS Graveyard :: Gaia::E-Mail, defect)
Firefox OS Graveyard
Gaia::E-Mail
Tracking
(blocking-b2g:-)
RESOLVED
INCOMPLETE
blocking-b2g | - |
People
(Reporter: freddy, Unassigned)
References
Details
(Keywords: sec-moderate, wsec-xss)
I've come to notice that there are stray bits and pieces of HTML sanitization code, e.g. in the mail parser:
./js/ext/mailparser/mailparser.js-1539-MailParser.prototype._joinHTMLAttachment = function(htmlNode, attachment){
./js/ext/mailparser/mailparser.js-1540- var inserted = false,
./js/ext/mailparser/mailparser.js-1541- fname = attachment.generatedFileName.replace(/</g, "<").replace(/>/g, ">").replace(/"/g, """),
./js/ext/mailparser/mailparser.js-1542- cid, newHTML;
./js/ext/mailparser/mailparser.js-1543-
./js/ext/mailparser/mailparser.js:1544: cid = attachment.cid || (attachment.cid = attachment.generatedFileName+"@node");
./js/ext/mailparser/mailparser.js-1545- newHTML = "\n<div class=\"mailparser-attachment\"><a href=\"cid:"+cid+"\"><" + fname + "></a></div>";
While it might be convenient to add a .replace().replace (line 1541), it's still incorrect: Escaping depends highly on the context this is being used in and you should have a centralized set of escape methods to be used in handling user input (e.g. attachment file names and CIDs). I suggest escapeforHTML, escapeforAttribute, escapeforURL.
I am setting this bug to security-sensitivealthough I could find no method to create an email that contains a malicious cid. It might just be me and the vulnerability is definitely there..
A cid that injects the class name "card" to the a object would make it use up the whole screen and hard to miss when touching the screen.
Comment 1•11 years ago
|
||
mailparser is an upstream lib that is currently only used for header parsing. The logic in question is not used because we get our structural information directly from the IMAP server's BODYSTRUCTURE results. We will, however, start to need mailparser's full functionality for POP3, although that might be the time for us to make a transition to Thunderbird's new JSMIME parser since that reduces Mozilla's net security surface area and results in various synergies as long as Thunderbird is a going concern.
I agree that the HTML handling there could be cleaned up. More significantly, I don't think we want any of the HTML touching that mailparser is doing. Most of that code is conflating display logic with parsing logic. (The charset logic might be okay? I would need to look into that more.)
Please note that all the code under js/ext/ is actually from the back-end repo https://github.com/mozilla-b2g/gaia-email-libs-and-more and that most of the bits there get concatenated. While it can be handy to grep in the 'ext' sub-tree because it does rule our files that are totally unused (our build step traces actually used dependencies at a file granularity), the line numbers are unreliable, etc.
Comment 3•11 years ago
|
||
Triage: This bug has been added to the v1.2 backlog but is not blocking release
Updated•10 years ago
|
Group: core-security
Updated•10 years ago
|
Group: b2g-core-security
Updated•9 years ago
|
Group: core-security → b2g-core-security
Comment 4•7 years ago
|
||
FirefoxOS is no longer under active development.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → INCOMPLETE
Updated•7 years ago
|
Group: b2g-core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•