New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 746132 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Not currently working on Chromium
Closed: Dec 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Chrome , Mac , Fuchsia
Pri: 1
Type: Bug-Security



Sign in to add a comment

bluetooth::mojom::AdapterFactory is available to any renderer without permission checks

Project Member Reported by sa...@chromium.org, Jul 19 2017

Issue description

ChromeContentBrowserClient::InitFrameInterfaces exposes bluetooth::mojom::AdapterFactory[1] to all RenderFrames. This interface provides full bluetooth access without permission checks. This interface is intended to only be used by webui.

[1] https://cs.chromium.org/chromium/src/chrome/browser/chrome_content_browser_client.cc?type=cs&q=ChromeContentBrowserClient::InitFrameInterfaces&sq=package:chromium&l=3288
 

Comment 1 by ortuno@chromium.org, Jul 19 2017

Cc: scheib@chromium.org cco3@chromium.org
Components: Blink>Bluetooth

Comment 2 by ortuno@chromium.org, Jul 19 2017

Description: Show this description

Comment 3 by och...@chromium.org, Jul 19 2017

Labels: -Restrict-View-Google
Cc: kerrnel@chromium.org
Labels: -M-61 Security_Severity-High M-59 Security_Impact-Head
Owner: ben@chromium.org
Status: Assigned (was: Untriaged)
That's a sandbox escape if all bluetooth access is available and should not be. Assigning to ben@ who added that code. It was added on June 20th, 2017 so I do not believe this change is in M-60, which branched on Thu, May 25, 2017 PT.
Labels: -M-59 M-61

Comment 6 by ortuno@chromium.org, Jul 19 2017

Cc: -scheib@chromium.org
Labels: -Security_Impact-Head -M-61 Security_Impact-Stable M-59
Owner: scheib@chromium.org
fwiw: ben's patch just moved things around without introducing any new bugs. The original patch landed in September of last year: https://crrev.com/2357383002. Assigning to scheib for triage.

Comment 7 by scheib@chromium.org, Jul 19 2017

Status: Started (was: Assigned)

Comment 8 by scheib@chromium.org, Jul 19 2017

Cc: ben@chromium.org
Project Member

Comment 9 by sheriffbot@chromium.org, Jul 26 2017

Labels: -M-59 M-60

Comment 10 by sa...@chromium.org, Aug 23 2017

Cc: jam@chromium.org
Project Member

Comment 11 by sheriffbot@chromium.org, Sep 6 2017

Labels: -M-60 M-61
Project Member

Comment 12 by sheriffbot@chromium.org, Sep 17 2017

Labels: Deadline-Exceeded
We commit ourselves to a 60 day deadline for fixing for high severity vulnerabilities, and have exceeded it here. If you're unable to look into this soon, could you please find another owner or remove yourself so that this gets back into the security triage queue?

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 13 by sheriffbot@chromium.org, Oct 18 2017

Labels: -M-61 M-62

Comment 14 by vakh@chromium.org, Nov 3 2017

scheib@ -- friendly ping from the security sheriff. can you please share an update on this high priority security bug? thanks.
There is a patch in progress.
scheib: any updates on the in-progress patch? Thanks!
Stalled since Nov 3 with non-code obligations. 
Components: Internals>Core UI>Browser>WebUI
Labels: -OS-All OS-Android OS-Chrome OS-Fuchsia OS-Linux OS-Mac OS-Windows
Friendly ping. :) We've blown our fix deadline, and the CL has been quiescent for 12 days. We really need to get this nailed down. Thank you!
Labels: -M-62 M-64
Patch needs cleanup and build rules scrutiny, but is progressing and now functional.  I've gotten it unblocked on the mojo changes that were blocking the refactor.
Cc: dpa...@chromium.org
Project Member

Comment 23 by bugdroid1@chromium.org, Dec 6 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dbe693beefc371f59a6561d6113ddc0c5af76889

commit dbe693beefc371f59a6561d6113ddc0c5af76889
Author: Vincent Scheib <scheib@chromium.org>
Date: Wed Dec 06 00:54:18 2017

bluetooth: internals page: Refactor instantiation of Bluetooth service.

Moves ability to create the //device/bluetooth/public/interfaces Adapter
to chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_handler.cc
instead of chrome/browser/chrome_content_browser_client.cc

//chrome/browser/ui/webui/bluetooth_internals target created to allow visibility
of //device/bluetooth/public/interfaces:deprecated_experimental_interfaces
to be restricted precisely to the single allowed client.

Numerous build files adjusted as needed to enforce that visibility restriction
and clearly document that the interfaces are only intended to be used by
bluetooth-internals.

Bug:  746132 
Cq-Include-Trybots: master.tryserver.chromium.linux:closure_compilation
Change-Id: Idcaa51d1ed4fb818bd3d8ae617e41a28eabfcb44
Reviewed-on: https://chromium-review.googlesource.com/775782
Reviewed-by: Demetrios Papadopoulos <dpapad@chromium.org>
Reviewed-by: Greg Kerr <kerrnel@chromium.org>
Commit-Queue: Vincent Scheib <scheib@chromium.org>
Cr-Commit-Position: refs/heads/master@{#521924}
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/BUILD.gn
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/browser_resources.grd
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/chrome_content_browser_client.cc
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/chrome_content_browser_manifest_overlay.json
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/resources/bluetooth_internals/adapter_broker.js
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/resources/bluetooth_internals/bluetooth_internals.html
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/BUILD.gn
[add] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/BUILD.gn
[add] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/OWNERS
[add] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals.mojom
[add] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_handler.cc
[add] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_handler.h
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_ui.cc
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/browser/ui/webui/bluetooth_internals/bluetooth_internals_ui.h
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/chrome/test/data/webui/bluetooth_internals_browsertest.js
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/device/BUILD.gn
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/device/bluetooth/BUILD.gn
[delete] https://crrev.com/9d0c82f471d7dfe177f059f9265f4919912823c1/device/bluetooth/adapter_factory.cc
[delete] https://crrev.com/9d0c82f471d7dfe177f059f9265f4919912823c1/device/bluetooth/adapter_factory.h
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/device/bluetooth/public/interfaces/BUILD.gn
[modify] https://crrev.com/dbe693beefc371f59a6561d6113ddc0c5af76889/device/bluetooth/public/interfaces/adapter.mojom

Status: Fixed (was: Started)
Project Member

Comment 25 by sheriffbot@chromium.org, Dec 7 2017

Labels: Restrict-View-SecurityNotify
Project Member

Comment 26 by sheriffbot@chromium.org, Dec 15 2017

Labels: Merge-Request-64
Project Member

Comment 27 by sheriffbot@chromium.org, Dec 15 2017

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: There is .grd file changes and we are only 38 days from stable.
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
abdulsyed@ - good for M64
Thanks awhalley@ and scheib@ - a large refactor worries me a bit at this stage. This also contains a grd change. Can we target this for M65 instead?
Labels: -Merge-Review-64 Merge-Rejected-64 M-65
Rejecting merge for M64. Seems like this has been around for a while (since July) and my recommendation is to wait until M65. 
Labels: -M-64
Labels: Release-0-M65
Project Member

Comment 33 by sheriffbot@chromium.org, Mar 15 2018

Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Sign in to add a comment