658691 - remove android:debuggable="true" from AndroidManifest.xml

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[:Atoll]

Consider removing android:debuggable="true" from AndroidManifest.xml, as otherwise private Firefox application data may be accessible without root access to all other applications.  From bug 592772#c50:

> I was considering the SharedPreferences again, and realized that the "run-as
> org.mozilla.fennec" way to access private data should not actually work in
> normal conditions! It does work for Fennec only because we have parameter
> android:debuggable="true" in the AndroidManifest.xml. If an app is not
> debuggable, its private data is not be accessible from the outside without
> root.

Flagging as a security issue since this puts us at risk of exposing private user data on non-rooted phones, if we shipped beta/final releases with this flag set.

Comment 1

[:Atoll]

cc alexp@ since he discovered this issue

Comment 2

[Alex Pakhotin (:alexp)]

Attachment: Patch

Set debuggable attribute to false.

Comment 3

[Doug Turner (:dougt)]

stealing review.  we should do this.

Comment 4

[Ian Melven :imelven]

i would really like to see this fixed asap also, fwiw.

Comment 5

[:Atoll]

What release of Fennec is the r+ patch above scheduled to ship in?

Comment 6

[Ian Melven :imelven]

the motivation behind wanting this fixed is stated in comment 50 of bug 592772 - " If an app is not debuggable, its private data is not be accessible from the outside without root. ".

also there's a much MUCH lower risk threat of someone not expecting the browser to be debuggable and not taking the proper precautions with connecting to usb and opting in to usb debugging - i mention it for completeness' sake :)

Comment 7

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 534148 [details] [diff] [review]
Patch

Fennec only

Comment 8

[Alex Pakhotin (:alexp)]

Attachment: Patch

Only updated commit message. Ready to be pushed.
r=dougt, a=mfinkle

Comment 9

[Ian Melven :imelven]

we should also perhaps consider documenting somewhere that fennec developers are going to want to manually turn this back on when actively developing fennec

Comment 10

[Brad Lassey [:blassey] (use needinfo?)]

Comment on attachment 539918 [details] [diff] [review]
Patch

please don't break all developers. This needs to at least be set to true for unofficial builds.

Comment 11

[Alex Pakhotin (:alexp)]

(In reply to comment #10)
> please don't break all developers. This needs to at least be set to true for
> unofficial builds.

It's possible, but I think it's not really such a big deal. It is so easy to enable debugging locally, we should just add this info to the debugging instructions on the Wiki (which the developers have to read anyway as it's impossible to debug without knowing some of those important bits). It does require rebuilding (repackaging) though.

What kind of scenario are you thinking about when this would cause a problem for developers? Do you mean some case when a binary installation is used and access to the files is needed?

Comment 12

[Brad Lassey [:blassey] (use needinfo?)]

Developers should be debugging all the time. Requiring them to make a code change in order to do it is overly onerous since it would have to be reverted before every update of your tree and reapplied after.

Comment 13

[Doug Turner (:dougt)]

makes sense.

Comment 14

[Mark Finkle (:mfinkle) (use needinfo?)]

Then let's add something to the mozconfig. Developers can set it in theirs, but not in the other builds. I don't want branding to be used as the flag.

Comment 15

[Ian Melven :imelven]

can we key off whether it's a debug build ? (i assume we're not shipping debug binaries via the android market, but it's an assumption :) )

ie. for a debug build (ac_add_options --enable-debug), debuggable will be true, for a build without this, it will be false 

we have a AndroidManifest.xml.in so i'm also assuming this is something configure can handle for us

Comment 16

[Alex Pakhotin (:alexp)]

Attachment: Patch v2

Keep debuggable flag for the unofficial build.

Comment 17

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/05a1cae42a22

Comment 18

[Mark Finkle (:mfinkle) (use needinfo?)]

http://hg.mozilla.org/releases/mozilla-aurora/rev/f98d15cfa9a5