Closed
Bug 959111
Opened 9 years ago
Closed 9 years ago
Connecting dev-tools to the Firefox OS remote debugging allows chrome script execution
Categories
(DevTools :: Debugger, defect)
Tracking
(blocking-b2g:koi+, firefox27 wontfix, firefox28 fixed, firefox29 fixed, firefox-esr24 unaffected, b2g18 unaffected, b2g-v1.2 fixed, b2g-v1.3 fixed, b2g-v1.4 fixed)
Tracking | Status | |
---|---|---|
firefox27 | --- | wontfix |
firefox28 | --- | fixed |
firefox29 | --- | fixed |
firefox-esr24 | --- | unaffected |
b2g18 | --- | unaffected |
b2g-v1.2 | --- | fixed |
b2g-v1.3 | --- | fixed |
b2g-v1.4 | --- | fixed |
People
(Reporter: arroway, Assigned: ochameau)
Details
(Keywords: sec-high, Whiteboard: [adv-main28-])
Attachments
(1 file, 1 obsolete file)
2.22 KB,
patch
|
ochameau
:
review+
|
Details | Diff | Splinter Review |
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.
Reporter | ||
Updated•9 years ago
|
Comment 1•9 years ago
|
||
I can confirm 1.1 release phones are unaffected. Would this be the status-b2g18 Tracking Flag?
Comment 2•9 years ago
|
||
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).
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.
blocking-b2g: --- → koi?
Keywords: sec-high
Summary: On a production locked phone, tools.debugger.forbid-certified-apps can be modified → Connecting dev-tools to the Firefox OS remote debugging allows chrome script execution
Updated•9 years ago
|
Assignee: nobody → poirot.alex
Comment 4•9 years ago
|
||
Alex will work on that, Panos will review the code. We hope to get a fix soon.
Assignee | ||
Comment 5•9 years ago
|
||
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
Updated•9 years ago
|
blocking-b2g: koi? → koi+
Assignee | ||
Updated•9 years ago
|
Attachment #8359315 -
Flags: review?(past)
Comment 6•9 years ago
|
||
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.
Attachment #8359315 -
Flags: review?(past) → review+
Comment 7•9 years ago
|
||
(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.
Assignee | ||
Comment 8•9 years ago
|
||
Updated comment and removed useless enable content actor code.
Attachment #8359315 -
Attachment is obsolete: true
Comment 9•9 years ago
|
||
(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•9 years ago
|
||
(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.
Assignee | ||
Comment 11•9 years ago
|
||
Note that I'll be using bug 958043 to clean things up around actors on b2g.
Assignee | ||
Comment 12•9 years ago
|
||
Comment on attachment 8359775 [details] [diff] [review] patch v2 Carrying r+ from previous patch.
Attachment #8359775 -
Flags: review+
Assignee | ||
Updated•9 years ago
|
Keywords: checkin-needed
Comment 13•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/caf64bb3f566
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/caf64bb3f566
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox29:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 29
Comment 15•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/48cb34aa7848 https://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/cb5001ce8b5e
status-firefox27:
--- → wontfix
status-firefox28:
--- → fixed
Do we have app manager support on 1.1.hd? I assume not, but I don't know too much about that branch.
(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.
Updated•9 years ago
|
status-b2g18:
--- → unaffected
status-firefox-esr24:
--- → unaffected
Updated•9 years ago
|
Whiteboard: [adv-main28-]
Updated•8 years ago
|
Group: core-security
Updated•5 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•