62.81 KB, image/png
355 bytes, text/html
This is security related. It should be a blocking-basecamp:+
Duplicate of this bug: 809247
from bug 809247 comment 0 With support for importing vcard files (.vcf) or from services like Facebook there is potential to exploit this without physical access to the device. Calling it sec-moderate if scripts are correctly being filtered out, but has anyone tried on* event handlers? If there's scripting of any kind this is probably sec-high
Created attachment 679321 [details] Screenshot of onload attribute setting the background color (In reply to Daniel Veditz [:dveditz] from comment #3) > Calling it sec-moderate if scripts are correctly being filtered out, but has > anyone tried on* event handlers? If there's scripting of any kind this is > probably sec-high on* event handlers are being evaluated: <img src="https://www.mozilla.org/favicon.ico" onload="this.style.backgroundColor='blue'"> changes the image background to blue.
Keywords: sec-moderate → sec-high
Applying the CSP should prevent the JS injections in on* event handlers.
(In reply to Fabrice Desré [:fabrice] from comment #5) > Applying the CSP should prevent the JS injections in on* event handlers. Sure, but we still shouldn't be parsing the HTML as it will cause problems when <>& are used in the fields.
Status: NEW → ASSIGNED
I will try to sanitize our contact fields before I put them in the DB.
Just serve as a sidenote: About CSP, there might still be minor issues after going with CSP. http://dvcs.w3.org/hg/content-security-policy/raw-file/tip/csp-1.0-specification.html Like A server transmits its security policy for a particular protected resource as a collection of directives, such as default-src 'self'... You can still inject <base ...> to finish the injection. (injection step for cross site attacking) So, please still be careful about handling all the tags if CSP applied.
(In reply to Gregor Wagner [:gwagner] from comment #7) > I will try to sanitize our contact fields before I put them in the DB. This is generally the opposite approach of what is recommended by OWASP for a few reasons: 1) The contact database shouldn't be tied to a single output format (HTML in this case). Suppose you want to implement json or vcard export: you wouldn't want to have to decode the HTML entities for that. 2) It will be a pain to alter the sanitization function once you already have contacts in the database because you would have to somehow migrate the old santized values to the new version. 3) Trusting what's in the database, by outputting it and assuming it's already been escaped is error prone and therefore may lead to security problem. e.g. importers from the SIM card, Facebook, etc. would all need to properly escape the HTML. It's safer to escape on output with an escapeHTML function already in Gaia.  http://stackoverflow.com/questions/2060923/sanitize-html-before-storing-in-the-db-or-before-rendering-antixss-library-in
OS: Linux → Gonk (Firefox OS)
Hardware: x86_64 → All
Priority: -- → P1
Target Milestone: --- → B2G C2 (20nov-10dec)
Gregor what's the risk of this change? What's your ETA for a fix?
Target Milestone: B2G C2 (20nov-10dec) → B2G C3 (12dec-1jan)
> > It's safer to escape on output with an escapeHTML function already in Gaia. > >  > http://stackoverflow.com/questions/2060923/sanitize-html-before-storing-in- > the-db-or-before-rendering-antixss-library-in That's fine with me. alberto can you fix this?
Created attachment 688505 [details] Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/6808 Pointer to Github pull-request
Thanks for the quick fix Alberto! Walter, Matthew can you run your test again with the pull request applied?
I ran my test again to see if it reproduce. I think it is fixed!
Attachment #688505 - Flags: review?(anygregor) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
status-firefox-esr10: --- → unaffected
status-firefox-esr17: --- → unaffected
verified in 20121212 build http://releases.mozilla.com/b2g/
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.