Closed Bug 850266 Opened 11 years ago Closed 9 years ago

Contacts API could allow theft of contact information.

Categories

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

x86_64
Windows 7
defect
Not set
normal

Tracking

(firefox40 unaffected, firefox41 unaffected, firefox-esr31 unaffected, firefox-esr38 unaffected)

RESOLVED FIXED
Tracking Status
firefox40 --- unaffected
firefox41 --- unaffected
firefox-esr31 --- unaffected
firefox-esr38 --- unaffected

People

(Reporter: codycrews00, Unassigned)

References

Details

(Keywords: csectype-disclosure, sec-moderate, Whiteboard: if enabled on Desktop this becomes sec-high)

Attached file contactpoc.html
User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:19.0) Gecko/20100101 Firefox/19.0
Build ID: 20130307023931

Steps to reproduce:

I initialized a contact and retrieved a reference to the prototype of the contact field, I then defined a setter for the 'value' property of the COW that is returned since the value is 'rw' in __exposedProps__.


Actual results:

This allows one to steal contact information as contacts are added until the browser is restarted.  This could be done for all types of contact fields this is just a very basic testcase showing the problem.  I know this doesn't pose a huge risk in desktop firefox, and have only been able to confirm that modifications to the prototypes do remain in the browser app on the b2g simulator/emulator.  Without a device to flash further testing could not be done.


Expected results:

Content level pages should be handed a wrapped version of the prototype when they request it or there should be multiple instances of the prototype.
Component: Untriaged → Gaia::Contacts
Product: Firefox → Boot2Gecko
QA Contact: isabelrios
Version: 19 Branch → unspecified
Very clever, cody.

So yeah. The issue here is that the contacts API has a shared prototype with writable properties in its __exposedProps__, which are presumably designed to be inherited. This should definitely not be allowed.

However, it's still not totally clear to me how the contacts can be stolen by cross-origin content. The cross-origin attacker can certainly get a reference to the contact with this technique, but that reference should be opaque because of the cross-origin-ness involved.

I'm guessing it would still be possible to leak information about the contact by passing it back to very chrome-implemented functions in the contact API, though.
Getting any information would be trivial as all you have to do is initialize another contact passing in the reference you hold as the name like so:

var tmpContact = new mozContact(); tmpContact.init({name:reference});

The chrome level String constructor will be called on the reference which will always have access to its toString method. So yeah nothing really stopping this from being fully utilized if except maybe the difference processes in b2g.
Forgive my terrible spelling and whatever that mess there is, I'm so sick right now I don't know what I'm doing.
I don't think this is an actual problem for b2g since we have the "process per app" model. An app needs the contacts privilege set in the manifest file which we check in the parent process and contacts can only be retrieved through the parent process. Note that the desktop version of b2g doesn't have the "process per app" mode enabled.

If we enable this API for desktop we might get into this problem but we haven't thought too much about the security implications on desktop for now.
Yeah I expected as much, actually found this about 2 months ago I think and didnt see the real need to disclose it.  One of the interesting things about using the contacts API in desktop FF though, is it loves to give back chrome level things, including arrays, date objects etc.  Right now the only thing this could be used for in desktop FF is exchanging objects cross domain through the prototypes or converting any Object you have a reference to into a string.
Status: UNCONFIRMED → NEW
Ever confirmed: true
Group: b2g-core-security
Keywords: sec-highsec-moderate
Whiteboard: if enabled on Desktop this becomes sec-high
Group: b2g-core-security
My head is exploding a little bit here. I'm definitely not understanding what this exploit allows. Or more importantly, why this is less bad in B2G where we're using child processes.

It's definitely bad even in a page is able to run Chrome JS. But it doesn't sound like that is possible?

If this enables pages that have access to one mozContact object to read arbitrary contact information then that's potentially also bad. It looks like we enable any page to instantiate mozContact objects and get access to the mozContact prototype chain. If that, and running in a process which has access to the full contacts API, is all that it takes to get access to the contacts database, then that's bad.
The code implementing the mozContact constructor has changed significantly since this, and I don't think you could see this work if you're testing it now.  I believe there's no longer just one instance of the prototype, and also that content never actually gets access to the real prototype anyway.  It's been some weeks since I went over the code behind this, but it's much more locked down.  There's also code implemented to use the Proxy API to hide the existence of __exposedProps__ even if one were to actually gain access to the "prototype"(actually a proxy representing it now).

Originally this showed that if the contacts API were to be used in desktop firefox that any malicious webpage could be stealing all the personal information of whoever you added.  I hope that helps clear things up, and I'm sure the B2G people can clear up anything I missed.
This was fixed by switching the contacts API to WebIDL in bug 850430, since there's no longer any __exposedProps__ to speak of. \o/
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → WORKSFORME
Group: core-security
(In reply to Gregor Wagner [:gwagner] from comment #4)
> I don't think this is an actual problem for b2g since we have the "process
> per app" model. An app needs the contacts privilege set in the manifest file
> which we check in the parent process and contacts can only be retrieved
> through the parent process. Note that the desktop version of b2g doesn't
> have the "process per app" mode enabled.

An app which has access to the contacts API is still able to open another website in an <iframe>. Or navigate to another website, for example if the user clicks an ad.

So we can't rely on the process boundary to fully protect us.
Group: b2g-core-security
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Jonas, comment 7 and comment 8 still apply, with WebIDL we no longer share the prototype instance. It doesn't even work locally with objects created by the Contacts API.

How would the app steal contact data from the website in the <iframe>?
Flags: needinfo?(jonas)
Yeah, comment 4 was just a mitigation analysis. The actual fix is per comment 8.
Ah, cool, we're good then.

I don't know if that happened late enough that it's safe to open this bug?
Status: REOPENED → RESOLVED
Closed: 10 years ago9 years ago
Flags: needinfo?(jonas)
Resolution: --- → FIXED
Now that ESR31 is EOLed, I think we're fine.

Over to Dan to open this back up.
Flags: needinfo?(dveditz)
Group: b2g-core-security
Depends on: 850430
Flags: needinfo?(dveditz)
Wondering if we have the same issue with other API that don't use WebIDL yet ? (I'm thinking of MobileMessaging for example -- but it's not in Desktop, only in B2G, and only for certified apps).
(In reply to Julien Wajsberg [:julienw] from comment #14)
> Wondering if we have the same issue with other API that don't use WebIDL yet
> ? (I'm thinking of MobileMessaging for example -- but it's not in Desktop,
> only in B2G, and only for certified apps).

It's already using webidl, no? http://mxr.mozilla.org/mozilla-central/source/dom/webidl/MozMobileMessageManager.webid
nsIDOMMozSmsMessage, etc aren't: http://mxr.mozilla.org/mozilla-central/source/dom/mobilemessage/interfaces/

I don't know if it's affected. Those message and thread objects are exposed via DOMRequest/DOMCursor.
In general, slaughterhouse makes that stuff much safer, but we should still get them moved over to WebIDL as soon as possible.
You need to log in before you can comment on or make changes to this bug.