1101158 - (CVE-2015-2745) Remote HTML tag injection in Gaia System app

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Muneaki Nishimura]

User Agent: Mozilla/5.0 (Windows NT 6.2; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/40.0.2214.6 Safari/537.36

Steps to reproduce:

1. Start Firefox OS 2.2 simulator from app-manager.
2. Open Browser app and launch following link.
http://alice.csrf.jp/http.php?s=308&u=https://www.google.co.jp/search?q=%3Cbase+href%3D%22http%3A%2F%2Fmallory.csrf.jp%2Ffake%2F%22%3E
3. Browser shows the search result of <base href="...">
4. Push HOME button and return to home screen
5. Long push HOME button and close Browser app window by 'x' button on the screen
6. Open Settings app and choose Find My Device
7. If a user doesn't enable the function, "Create account or sign in" button is shown on the screen.
8. Click the "Create account or sign in" button



Actual results:

http://mallory.csrf.jp/fxa/fxa_module.html is shown instead of legitimate Firefox account creation page.



Expected results:

/fxa/fxa_module.html in the application is shown.

Comment 1

[Muneaki Nishimura]

It seems that this cause is handling of {title} in following code.
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L74

Here, {title} is not escaped properly, so <base> tag can be injected via application's title when a user shows cards-view by long pressing HOME button.

Comment 2

[Muneaki Nishimura]

In addition to above my comment, {title} may be set here.
https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/card.js#L112

If you add HTML escaping for app.title and app.name, this vulnerability may be resolved.

Comment 3

[Muneaki Nishimura]

In order to reduce similar risk, please consider to follow W3C's spec that allows to use base element in a head element.
http://www.w3.org/html/wg/drafts/html/master/document-metadata.html#the-base-element

IE is compliant with the spec and it ignores base elements in HTML body.

Comment 4

[Muneaki Nishimura]

By using same issue, I found a way to embed arbitrary application like below. 

1. Start Firefox OS 2.2 simulator from app-manager.
2. Open Browser app and launch following link.
http://alice.csrf.jp/http.php?s=308&u=https://www.google.co.jp/search?q=%3Ciframe+mozbrowser+remote+mozapp%3D%22app%3A%2F%2Ffm.gaiamobile.org%2Fmanifest.webapp%22+src%3D%22app%3A%2F%2Ffm.gaiamobile.org%2Findex.html%22%3E
3. Browser shows the search result of <iframe...>
4. Long push HOME button, then, built-in FM Radio app is launched on card-view.

Comment 5

[Muneaki Nishimura]

I reproduced <iframe mozapp> injection not only on Firefox 2.2 simulator but also on Firefox OS 2.1 device.

Comment 6

[Daniel Veditz [:dveditz]]

Is this only working in the simulator? I'm trying it a desktop b2g build and can't reproduce. Then again I'm having some trouble with the home button so it could just be this build.

Comment 7

[Muneaki Nishimura]

I've ever confirmed this bug on following environment.

Launching Firefox Account sign-in page:
 --> FxOS 2.2 simulator / real device. FxOS 2.1 is not affected.

Injecting other application on card-view with iframe[mozapp]:
 --> FxOS 2.1 and 2.2 simulator. FxOS 2.0 is not affected.

Comment 8

[Muneaki Nishimura]

Hi, same bug can be reproduced by following procedure.

1. Open following URL by Search app and show the search result page.
https://www.google.co.jp/search?q=%3Ciframe%20mozbrowser%20remote%20mozapp=%22app://fm.gaiamobile.org/manifest.webapp%22%20src=%22app://fm.gaiamobile.org/index.html%22%3E
2. Push "..." button on the left side of location bar.
3. Push "Show Windows".
4. FM application is embedded on the tab list page.

Note that as you know System app has 'embed-apps' and 'webapps-manage' permissions, so it can launch any apps with privileged or certified privilege.

Comment 9

[Frederik Braun [:freddy]]

I could repro comment 8 on a 2.2 Simulator.

Comment 10

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Daniel Veditz [:dveditz] from comment #6)
> Is this only working in the simulator? I'm trying it a desktop b2g build and
> can't reproduce. Then again I'm having some trouble with the home button so
> it could just be this build.

Freddy will have a look at this while I am away.

Comment 11

[Muneaki Nishimura]

I found another attack scenario by using this bug.
This is a way to bypass SSL cert error with click-jacking on card-view.
(Note that following PoC code is adjusted to Firefox OS Simulator v2.2.)

1. Launch following URL on Browser.
https://www.google.co.jp/search?q=%3Ciframe+mozbrowser+src%3D%22https%3A%2F%2Fbob.csrf.jp%2F%22+style%3D%22position%3Afixed%3Bz-index%3A100%3Btop%3A-330px%3Bleft%3A-100px%3Bwidth%3A320px%3Bheight%3A480px%3Bborder-width%3A0px%3Bopacity%3A1.0%3B%22%3E%3C%2Fiframe%3E%3Cimg+src%3D%22http%3A%2F%2Falice.csrf.jp%2Ffxos%2Fphishing.png%22+style%3D%22position%3Afixed%3Bz-index%3A101%3Btop%3A-130px%3Bleft%3A-85px%3Bwidth%3A320px%3Bheight%3A400px%3Bopacity%3A0.3%3Bpointer-events%3Anone%3B%22%3E

2. Long press HOME button and open card-view.
3. Here, you can find "Click Here" buttons on SSL cert error page.
4. Click "Click Here" buttons from 1->2.
5. Then, SSL cert error for https://bob.csrf.jp/ can be ignored permanently on the device.
And this ignorance is affected to all applications on the device.

This scenario is applicable not only for System app but also for all apps declaring "browser" permission.
If you consider mitigation in platform level, SSL cert error page should show modal confirmation dialog before applying "Add permanent exception".

Comment 12

[Frederik Braun [:freddy]]

This exploit is confirmed to work on devices.

CSP defends against scripting, but as comment 11 shows, some nasty things are still possible (styling, i.e., clickjacking and mis-using the authority of the browser app to bypass warnings globally)

Comment 13

[Frederik Braun [:freddy]]

Kevin, you're a System app peer. Can you help getting this assigned?
Comment 1 and comment 2 show the lines that need patching.

Comment 14

[Kevin Grandon :kgrandon]

This is a bit silly of us for continually trying to re-invent the wheel. The system app already includes template.js, so maybe we can leverage that.

I'll either look for an owner today, or submit a fix. Thanks.

Comment 15

[Kevin Grandon :kgrandon]

Feels like a system issue to me, so moving components. I'm also looking at this now. Thanks.

Comment 16

[Kevin Grandon :kgrandon]

Attachment: [Patch] Use new shared tagged template library to escape card HTML

Let's try something new! This introduces a simple shared library for managing tagged template strings. For this to be really useful it's need the concept of more types of escaping, and probably the ability to whitelist vars. For this use case, and as something to start us off with, I think we can land something like this.

This does have a dependency on the linter being upgraded to fully support template literals. I'm tracking this in bug 1098149.

I do think this is ready to be reviewed though. Adding :aus and :freddyb as potential reviewers. Aus being a task manager expert, and freddy since he did a talk about tagged template strings. Thanks guys!

Comment 17

[Frederik Braun [:freddy]]

Comment on attachment 8527769 [details] [diff] [review]
[Patch] Use new shared tagged template library to escape card HTML

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

Looking good. Comments below.

::: apps/system/js/card.js
@@ +90,5 @@
> +        data-button-action="favorite" role="button" 
> +        style="visibility: ${this.favoriteButtonVisibility}"></button>
> +     </menu>
> +    </footer>`;
> +  };

This hunk introduces some trailing whitespaces

::: shared/js/tagged.js
@@ +11,5 @@
> +    '&': '&amp;',
> +    '<': '&lt;',
> +    '>': '&gt;',
> +    '"': '&quot;',
> +    '\'': '&apos;'

Can we add the forward-slash character to the list of entities as well as the regular expression? 
(See https://www.owasp.org/index.php/XSS_%28Cross_Site_Scripting%29_Prevention_Cheat_Sheet#RULE_.231_-_HTML_Escape_Before_Inserting_Untrusted_Data_into_HTML_Element_Content for context)

Comment 18

[Aus Lacroix [:aus]]

Comment on attachment 8527769 [details] [diff] [review]
[Patch] Use new shared tagged template library to escape card HTML

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

Looks good to me, one comment, somewhat of a nit. I'll let you decide if it's worth it. :)

::: apps/system/js/card.js
@@ +97,5 @@
>     * Card html view - builds the innerHTML for a card element
>     * @memberOf Card.prototype
>     */
>    Card.prototype.view = function c_view() {
> +    return this.template();

This is kind of a nit but, all other Views we have in existence have 'template' as an object property. This one changes it to a function. It would be pretty easy to make it a getter instead so we keep the same compatibility with other Views.

Comment 19

[Kevin Grandon :kgrandon]

Comment on attachment 8527769 [details] [diff] [review]
[Patch] Use new shared tagged template library to escape card HTML

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

::: apps/system/js/card.js
@@ +97,5 @@
>     * Card html view - builds the innerHTML for a card element
>     * @memberOf Card.prototype
>     */
>    Card.prototype.view = function c_view() {
> +    return this.template();

I think I'm going to leave this for now, but I'm more than happy to follow this up and fix it if that's a better design. I couldn't really find locations where we had a template getter, and it's a bit of a pain to do the defineGetter on the prototype. Can you show me the other places that we have the template property?

Comment 20

[Kevin Grandon :kgrandon]

I am going to land this now, and I want to start promoting this in more places. Unfortunately this is going to be difficult due to jshint not yet supporting tagged template strings. I've filed an issue and will try to help move it along: https://github.com/jshint/jshint/issues/2000

Comment 21

[Kevin Grandon :kgrandon]

In master: https://github.com/mozilla-b2g/gaia/commit/0f72a8c31df9f3d8f609a7c58ca91139051d44eb

Comment 22

[Frederik Braun [:freddy]]

We want this in 2.1 too, right?

Comment 23

[Kevin Grandon :kgrandon]

[Blocking Requested - why for this release]:

(In reply to Frederik Braun [:freddyb] from comment #22)
> We want this in 2.1 too, right?

Ah, I thought this was only 2.2, but based on comment 7, I see that it impacts v2.1 as well:

> Injecting other application on card-view with iframe[mozapp]:
> --> FxOS 2.1 and 2.2 simulator. FxOS 2.0 is not affected.

Updating the tracking flags to reflect this, and also requesting blocking.

Comment 24

[Muneaki Nishimura]

Attachment: reproduced on 2.1

Attached is a screenshot when I reproduced this bug on FxOS 2.1 simulator.

Comment 25

[Ryan VanderMeulen [:RyanVM]]

Per the landing rules, this has auto-approval for uplift to v2.1 since it's sec-high. That said, it needs rebasing first :)

Comment 26

[Kevin Grandon :kgrandon]

Thanks, I wasn't aware of that.

Comment 27

[Kevin Grandon :kgrandon]

Attachment: Pull request - Uplift to 2.1

Here is the rebased patch.

Comment 28

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/5655269098c7e82254e56933f1af05b4abe2a2f3

Comment 29

[Muneaki Nishimura]

Please let me know how long I should keep the bug a secret?
I'd like to introduce it to web developers for explaining how HTML injection is bad.

There may be less impact even if I disclose because it only affects unstable version of B2G and as far as I know official firmware with the fix has been distributed for Frame.

Comment 30

[Frederik Braun [:freddy]]

You are right, there are no end-users affected, which is a really good thing.

Unfortunately, a lot of Firefox OS users (especially inside Mozilla) use an unstable version of B2G as their production phone, to get the latest/best experience and also to find bugs before we release the software.

I *do* want to enable you to talk about this bug, but please allow us to think about this for a while.

Comment 31

[Muneaki Nishimura]

Frederik, thanks.How about the status? Concluded internally?

Comment 32

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to Muneaki Nishimura from comment #31)
> Frederik, thanks.How about the status? Concluded internally?

Sorry Muneaki, there are two devices that use 2.1 that I am still confirm have taken the patch. They are not released but I'm not sure what stage of development they are at. Joe could you please confirm once you have spoken to partners and we get the all clear? Thanks,

Comment 33

[Muneaki Nishimura]

Do you have any info about comment #32?
I wonder if you'd permit me to make it public on Dec-20.

Comment 34

[howie [:howie]]

Hi Muneaki, please wait for a while after all the communication to partners is cleared.

@ Vance, could you help to check with partners that use 2.1 have this patch taken? Thank you.

Comment 35

[shawn ku [:sku]]

Hi Muneaki/Paul:
As I know, there is only one partner will take 2.1 (that said 2.1s) now, and not branched out yet.
Ryan already push the patch to 2.1 per comment 28. I think we should all be fine here.

However, there are two devices (in comment 32) use 2.1 as Paul said which is out of my radar.
Please kindly let me know which of two.


Hi Vance,
 please shed some lights for partners branch things if I miss anything here.

Comment 36

[Vance Chen [:vchen][skype:vance.lucida][[e-mail]]]

Hi all -

Already clarify this offline with Paul. Right now only SPRD will take 2.1. And as Shawn mentioned in Comment#35, the 2.1s(specifically for SPRD) branch is not yet generated, so we are good here. And of course some other partners might take 2.1 as well in the future, but that won't be a problem too since we already patch the fix into 2.1

Thanks and please let me know shall you have further concern

Vance

Comment 37

[Muneaki Nishimura]

Hi all, thanks for handling my request though you may have higher priority tasks.
Could someone can remove core-security flag if no further concern are found.
Happy holidays!

Comment 38

[Paul Theriault [:pauljt] (no longer reading bugmail)]

(In reply to shawn ku [:sku] from comment #35)
> Hi Muneaki/Paul:
> As I know, there is only one partner will take 2.1 (that said 2.1s) now, and
> not branched out yet.
> Ryan already push the patch to 2.1 per comment 28. I think we should all be
> fine here.
> 
> However, there are two devices (in comment 32) use 2.1 as Paul said which is
> out of my radar.
> Please kindly let me know which of two.

I responded via email, its probably fine,  but there are two devices I want to check.

Thanks for you patience Muneaki, I'll respond asap with more info.

Comment 39

[Paul Theriault [:pauljt] (no longer reading bugmail)]

Vance confirmed the remaining two devices via email. Opening this bug up.

Comment 40

[Elie]

Attachment: VIDEO0174_Compress.MP4

This issue has been successfully verified on Flame 2.1:
Gaia-Rev        6af3d029bae3a14f400fec0926f0f8ad7b579b4b
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/39eaa987093b
Build-ID        20141221001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2 
Refer to video

Comment 41

[Annabelle]

Attachment: Verified_Flame_v2.2.3gp

This bug has been verified as "pass" on latest build of Flame 2.2 by STR in comment 0 and comment 4.

Actual result(by STR of comment 0): The device will jump to creat Firefox Accounts view after you kill the Browser by tapping "X" in card-view.
Actual result(by STR of comment 4): No other app is launched in card-view.
See video: Verified_Flame_v2.2.3gp
Repreduce rate: 0/5

Device: Flame v2.2(Pass)
Build ID               20150720002503
Gaia Revision          e1e6317f17a840b19af9dbb25f5a771d8d9fa161
Gaia Date              2015-07-15 21:05:11
Gecko Revision         https://hg.mozilla.org/releases/mozilla-b2g37_v2_2/rev/e2d1f1f55803
Gecko Version          37.0
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150720.042247
Firmware Date          Mon Jul 20 04:22:58 EDT 2015
Bootloader             L1TC000118D0