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)

ARM
Gonk (Firefox OS)
defect
Not set
normal

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)

Attached image reproduced.png
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.
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.
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.
Attached patch Patch 1 (obsolete) — Splinter Review
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.
Assignee: nobody → stas
Status: NEW → ASSIGNED
Attachment #8642437 - Flags: review?(gandalf)
(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 on attachment 8642437 [details] [diff] [review]
Patch 1

Review of attachment 8642437 [details] [diff] [review]:
-----------------------------------------------------------------

Great catch Muneaki!
Attachment #8642437 - Flags: review?(gandalf) → review+
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)
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)
Landed on master: https://github.com/mozilla-b2g/gaia/commit/65c731fbe7023a5d27badb48a0f1b27f9170cfb5
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
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?
Ryan, would you mind helping us with the uplift to 2.2 here?
Flags: needinfo?(ryanvm)
Doesn't cherry-pick cleanly, please rebase. No need to ni? me when you're done.
Flags: needinfo?(ryanvm) → needinfo?(stas)
Target Milestone: --- → FxOS-S4 (07Aug)
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
Flags: needinfo?(stas)
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'
Attachment #8642642 - Flags: approval-gaia-v2.2?
See Also: → 1196014
Group: core-security → core-security-release
Whiteboard: [b2g-adv-main2.5+]
Alias: CVE-2015-8510
Group: core-security-release
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: