959111 - Connecting dev-tools to the Firefox OS remote debugging allows chrome script execution

Status: RESOLVED FIXED

(DevTools :: Debugger, defect, critical)

Description

[Stephanie Ouillon [:arroway] (needinfo me)]

You need physical access to perform the following :
1. Start the server debugger on a production locked phone
2. Connect with the connection screen in the devtools
3. You now have access to the main console via a web console, with a root js console.
4. You can change the settings tools.debugger.forbid-certified-apps from true to false

Firefox OS 1.2 to 1.4 are impacted.

Comment 1

[Frederik Braun [:freddyb]]

I can confirm 1.1 release phones are unaffected. Would this be the status-b2g18 Tracking Flag?

Comment 2

[Paul Rouget [:paul]]

More detailed STR:

1. in Gaia setting, check "Remote Debugger"
2. on a laptop, "adb forward tcp:6000 localfilesystem:/data/local/debugger-socket"
3. In Firefox Desktop, turn on the "devtools.debugger.remote-enabled" pref to True
4. Load this page: chrome://browser/content/devtools/connect.xhtml
5. Connect to localhost:6000
6. Click "Main Process"

The console that shows up is connected to shell.js (top root process).

Comment 3

[Paul Theriault [:pauljt] (needinfo pls)]

Renaming since the issue here is that remote debugging service on the phone allows access to connect to chrome (shell.js). Changing the tools.debugger.forbid-certified-apps pref is just one thing you can do, but iiuc you have chrome script, ie root access. 

The attack is trivial, but you do need local access to the phone. Having a PIN set on the lockscreen will prevent this attack since the remote debugger will not connect unless the lockscreen is unlocked. 

From a risk standpoint I would say this either sec-high or sec-critical. Its local only, but it is somewhat equivalent to shipping a phone with adb running as root, which makes me think this really should be sec-critical. We really should get this in 1.2 if at all possible at this point.

Comment 4

[Paul Rouget [:paul]]

Alex will work on that, Panos will review the code. We hope to get a fix soon.

Comment 5

[Alexandre Poirot [:ochameau]]

Attachment: Prevent loading tab actors in parent process unless certified app pref is toggled

In 1.1 we had no real devtools support, so we weren't loading tab actors in parent process on device
(ie we were loading it on desktop so that we can at least debug shell.xul).

In 1.2 we started enabling app debugging, and, in order to debug apps living in parent process (system and keyboard apps),
we had to load tab actors (console, inspector, debugger, ...) also in parent process [1].
The side effect of that, is that the root actor was also exposing tab actors for the main process,
even if we took care of preventing it to expose actors for the toplevel windows [2].

Ideally we would also explicitely prevent the root actor to expose the tab actors for main process,
even if the actors are loaded. But in this patch I just prevent them to be loaded, unless
the pref to allow certified apps debugging is set.

[1]
  http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/shell.js#1075
[2]
  http://mxr.mozilla.org/mozilla-central/source/b2g/chrome/content/dbg-browser-actors.js#30

Comment 6

[Panos Astithas (he/him) [:past] (please ni?)]

Comment on attachment 8359315 [details] [diff] [review]
Prevent loading tab actors in parent process unless certified app pref is toggled

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

This looks good to me. The only observation I want to make is that if the debugger is already started and the user changes the forbid-certified-apps pref, stopping and starting remote debugging would be required for the actors to be loaded, right? We may want to mention that in the docs.

::: b2g/chrome/content/shell.js
@@ +1072,5 @@
>        DebuggerServer.init(this.prompt.bind(this));
>        DebuggerServer.chromeWindowType = "navigator:browser";
>        DebuggerServer.addActors("resource://gre/modules/devtools/server/actors/webbrowser.js");
>        // Until we implement unix domain socket, we enable content actors
>        // only on development devices

Nit: this comment is no longer accurate. Better remove or reword it.

Also, if you feel like it, there is a similar comment in the webapps.js actor definition that needs an update or removal.

But in general, I'm not clear on whether enable-content-actors is still useful now that UNIX domain sockets are used. I don't mean to imply this is something that belongs to this bug, just wanted to raise the issue.

Comment 7

[Paul Rouget [:paul]]

(In reply to Panos Astithas [:past] from comment #6)
> This looks good to me. The only observation I want to make is that if the
> debugger is already started and the user changes the forbid-certified-apps
> pref, stopping and starting remote debugging would be required for the
> actors to be loaded, right? We may want to mention that in the docs.

There's no way to change the pref when fxos is running.

> But in general, I'm not clear on whether enable-content-actors is still
> useful now that UNIX domain sockets are used. I don't mean to imply this is
> something that belongs to this bug, just wanted to raise the issue.

This will disappear in bug 942756.

Comment 8

[Alexandre Poirot [:ochameau]]

Attachment: patch v2

Updated comment and removed useless enable content actor code.

Comment 9

[Panos Astithas (he/him) [:past] (please ni?)]

(In reply to Paul Rouget [:paul] from comment #7)
> (In reply to Panos Astithas [:past] from comment #6)
> > This looks good to me. The only observation I want to make is that if the
> > debugger is already started and the user changes the forbid-certified-apps
> > pref, stopping and starting remote debugging would be required for the
> > actors to be loaded, right? We may want to mention that in the docs.
> 
> There's no way to change the pref when fxos is running.

What about pulling prefs.js with adb, editing and pushing it back? Isn't this something developers would be accustomed to?

Comment 10

[Paul Rouget [:paul]]

(In reply to Panos Astithas [:past] from comment #9)
> (In reply to Paul Rouget [:paul] from comment #7)
> > (In reply to Panos Astithas [:past] from comment #6)
> > > This looks good to me. The only observation I want to make is that if the
> > > debugger is already started and the user changes the forbid-certified-apps
> > > pref, stopping and starting remote debugging would be required for the
> > > actors to be loaded, right? We may want to mention that in the docs.
> > 
> > There's no way to change the pref when fxos is running.
> 
> What about pulling prefs.js with adb, editing and pushing it back? Isn't
> this something developers would be accustomed to?

In the documentation, way explicitly ask the user to restart B2G via adb shell.

Comment 11

[Alexandre Poirot [:ochameau]]

Note that I'll be using bug 958043 to clean things up around actors on b2g.

Comment 12

[Alexandre Poirot [:ochameau]]

Comment on attachment 8359775 [details] [diff] [review]
patch v2

Carrying r+ from previous patch.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/fx-team/rev/caf64bb3f566

Comment 14

[Wes Kocher (:KWierso)]

https://hg.mozilla.org/mozilla-central/rev/caf64bb3f566

Comment 15

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/48cb34aa7848
https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/cb5001ce8b5e

Comment 16

[Paul Theriault [:pauljt] (needinfo pls)]

Do we have app manager support on 1.1.hd? I assume not, but I don't know too much about that branch.

Comment 17

[J. Ryan Stinnett [:jryans]]

(In reply to Paul Theriault [:pauljt] from comment #16)
> Do we have app manager support on 1.1.hd? I assume not, but I don't know too
> much about that branch.

No, only 1.2+ is supported by the App Manager.