Closed
Bug 1190038
(CVE-2015-8510)
Opened 9 years ago
Closed 9 years ago
HTML injection on homescreen app (with bypassing DOM sanitizer)
Categories
(Firefox OS Graveyard :: Gaia, defect)
Tracking
(b2g-v2.1S unaffected, b2g-v2.2 fixed, b2g-v2.2r fixed, b2g-master fixed)
RESOLVED
FIXED
FxOS-S4 (07Aug)
Tracking | Status | |
---|---|---|
b2g-v2.1S | --- | unaffected |
b2g-v2.2 | --- | fixed |
b2g-v2.2r | --- | fixed |
b2g-master | --- | fixed |
People
(Reporter: sdna.muneaki.nishimura, Assigned: stas)
References
Details
(Keywords: sec-high, wsec-xss, Whiteboard: [b2g-adv-main2.5+])
Attachments
(2 files, 1 obsolete file)
46.25 KB,
image/png
|
Details | |
5.02 KB,
patch
|
stas
:
review+
|
Details | Diff | Splinter Review |
1. Prepare a device that Firefox OS 2.5.0.0-prelease is installed. I used my Flame with a firmware id 201507311030207. 2. Launch Browser and open http://csrf.jp/manifest/xss.html . 3. Push 'Add to home screen' button and 'Add' button. Then a bookmark with a lock icon is created. 4. Long press the lock icon and try to uninstall the bookmark. 5. As attached picture, a confirmation dialog is shown with FM Radio application (if reproduced). At step 2, the page sets "<s>PoC<iframe src=app://fm.gaiamobile.org/index.html mozbrowser></iframe>" as it's application name throught the web manifest. When I set iframe tag simply like "<iframe src=app://fm.gaiamobile.org/index.html mozbrowser></iframe>" as an application name then the iframe is correctly removed by DOM sanitizer in Gaia. But above text pattern, i.e., <s> + <iframe>, can bypass DOM sanitizer and the iframe is executed with homescreen app's privilege.
Updated•9 years ago
|
Flags: sec-bounty?
Ben - this looks it is exploited via w3c app support. Can you have a look here? The vuln is probably somewhere else but we may have violated an assumption here.
Actually the strings for this issue are here: https://github.com/mozilla-b2g/gaia/blob/master/apps/system/locales/system.en-US.properties#L288 I wonder if this is another bug in our l20n code, since I _think_ this has a sanitizer in it.
FWIW, this bug has been recently introduced since I had to flash from dogfood to dogfood-latest to reproduce this.
Assignee | ||
Comment 4•9 years ago
|
||
I can reproduce this and I can confirm that this is a bug in l10n.js/l20n.js. We only recursively sanitize the translation if the outer element was found in the source HTML: https://github.com/mozilla-b2g/gaia/blob/8dba2077f5e7137253fbb3faf10cd0b5f7da25c2/shared/js/l10n.js#L2006 In the test case, <s> is not in the source HTML, so l10n.js doesn't sanitize its children and appends the whole thing to DOM. I'll fix this.
Assignee | ||
Comment 5•9 years ago
|
||
This patch calls the sanitization logic recursively on children of allowed elements which are only present in translations. I'm not sure what the disclosure policy is so I haven't submited a pull request yet. Also, I think this bug might be present on 2.2 as well, because bug 994357 was backported. https://github.com/mozilla-b2g/gaia/commit/3c71bd67292bbb9f898d1045f07cbe0b4e565762 I'll also add a test before I'm ready to land.
Comment 6•9 years ago
|
||
(In reply to Paul Theriault [:pauljt] from comment #2) > I wonder if this is another bug in our l20n code, since I _think_ this has a > sanitizer in it. Do we have other bugs in our l20n code? I don't know of any.
Comment 7•9 years ago
|
||
Comment on attachment 8642437 [details] [diff] [review] Patch 1 Review of attachment 8642437 [details] [diff] [review]: ----------------------------------------------------------------- Great catch Muneaki!
Attachment #8642437 -
Flags: review?(gandalf) → review+
Assignee | ||
Comment 8•9 years ago
|
||
Paul, what's the best way of landing this on master and 2.2? Do we have users on 2.2 that we should be careful about here? Should I simply create a pull request?
Flags: needinfo?(ptheriault)
Assignee | ||
Comment 9•9 years ago
|
||
Added a test. Carrying over the r+. I'm going to sleep and will check back in 7-8 hours. If you need to land this before then, this is ready to go. Thanks!
Attachment #8642437 -
Attachment is obsolete: true
Attachment #8642642 -
Flags: review+
(In reply to Staś Małolepszy :stas from comment #8) > Paul, what's the best way of landing this on master and 2.2? Do we have > users on 2.2 that we should be careful about here? Should I simply create a > pull request? I just checked and we don't currently have any devices on 2.2, so I think its ok to land.
Flags: needinfo?(ptheriault)
Assignee | ||
Comment 11•9 years ago
|
||
Landed on master: https://github.com/mozilla-b2g/gaia/commit/65c731fbe7023a5d27badb48a0f1b27f9170cfb5
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8642642 [details] [diff] [review] Patch 2 with a test [Approval Request Comment] [Bug caused by] (feature/regressing bug #): bug 994357 [User impact] if declined: XSS is possible via well-prepared name of app, or name of bluetooth device, or name of wifi network (if they're used in translations) [Testing completed]: tested on device; added a unit test [Risk to taking this patch] (and alternatives if risky): very low [String changes made]: none
Attachment #8642642 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Assignee | ||
Comment 13•9 years ago
|
||
Ryan, would you mind helping us with the uplift to 2.2 here?
Flags: needinfo?(ryanvm)
Comment 14•9 years ago
|
||
Doesn't cherry-pick cleanly, please rebase. No need to ni? me when you're done.
status-b2g-v2.1S:
--- → unaffected
status-b2g-v2.2:
--- → affected
status-b2g-v2.2r:
--- → affected
status-b2g-master:
--- → fixed
Flags: needinfo?(ryanvm) → needinfo?(stas)
Target Milestone: --- → FxOS-S4 (07Aug)
Comment 15•9 years ago
|
||
The conflicts were that l20n.js doesn't exist on v2.2. Stas confirmed that it's safe to ignore those changes and I went ahead and pushed the rest. v2.2: https://github.com/mozilla-b2g/gaia/commit/102f1299e9eafe3760e1deb44d556b5c4f36b5af
Updated•9 years ago
|
Flags: sec-bounty? → sec-bounty+
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #6) > (In reply to Paul Theriault [:pauljt] from comment #2) > > I wonder if this is another bug in our l20n code, since I _think_ this has a > > sanitizer in it. > > Do we have other bugs in our l20n code? I don't know of any. Late response - I meant, "I wonder if this bug is another issue similar to bug 1190137, 1190139 etc'
Updated•9 years ago
|
Attachment #8642642 -
Flags: approval-gaia-v2.2?
Updated•9 years ago
|
Group: core-security → core-security-release
Updated•9 years ago
|
Whiteboard: [b2g-adv-main2.5+]
Updated•8 years ago
|
Alias: CVE-2015-8510
Updated•8 years ago
|
Group: core-security-release
You need to log in
before you can comment on or make changes to this bug.
Description
•