New issue
Advanced search Search tips
Starred by 2 users
Status: Fixed
Owner:
Closed: Jul 12
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: service_manager{client_process} Capability Not Properly Enforced
Project Member Reported by tjbecker@google.com, Jul 10 Back to list
VULNERABILITY DETAILS

A compromised process (renderer, utility, etc) can bypass the service manager's manifest enforcement and/or impersonate other services.

In services/service_manager/service_manager.cc, Instance::StartServiceWithProcess fails to enforce the client_process capability, because it passes nullptr into the validation function:

  void StartServiceWithProcess(
      const Identity& in_target,
      mojo::ScopedMessagePipeHandle service_handle,
      mojom::PIDReceiverRequest pid_receiver_request,
      StartServiceWithProcessCallback callback) override {
    Identity target = in_target;
    mojom::ConnectResult result =
        ValidateConnectParams(&target, nullptr, nullptr); <-- should pass service_handle and receiver_request
    ...
    ...
    service_manager_->Connect(std::move(params));
  }

The target identity is still validated by ValidateConnectionSpec(), but it allows the target user_id to be kRootUserID and the target service to be any service in the source's "requires" list. If the target passes this validation, ServiceManager::Connect() will be called, which will call CreateInstance(). A new Instance (with the target identity) will be created and bound to the service provided by the compromised process.

Exploit Strategy:
A compromised renderer can call StartServiceWithProcess with Identity(kRootUserID, "content_browser") to create a fake "content_browser" service running in its process. Using this new service's Connector, the renderer process can now access sensitive services (e.g. file service, preferences service, etc.).
It can also replace existing services with fake ones to intercept potentially sensitive IPC calls. The fake "content_browser" service has capabilities to create services with any user id and instance name:

  "service_manager": [
    "service_manager:client_process",
    "service_manager:instance_name",
    "service_manager:service_manager",
    "service_manager:user_id"
  ],

Note: This works even if the target identity was already created, because the following insertion will fail silently on production builds:

  auto result =
      identity_to_instance_.insert(std::make_pair(target, raw_instance));
  DCHECK(result.second);

VERSION
Chrome Version: stable
Operating System: All

REPRODUCTION CASE
Working on a PoC now, which I'll post soon.
 
Cc: tsepez@chromium.org jam@chromium.org ben@chromium.org
Labels: ReleaseBlock-Stable Security_Severity-High Security_Impact-Stable Pri-1
Owner: roc...@chromium.org
Status: Assigned
FYI fix with test case at https://chromium-review.googlesource.com/c/566479/
Project Member Comment 3 by sheriffbot@chromium.org, Jul 11
Labels: M-59
Project Member Comment 4 by sheriffbot@chromium.org, Jul 11
Labels: Hotlist-Google
Attached a PoC exhibiting arbitrary file r/w to the user's profile directory from a compromised renderer. This could likely be used to achieve arbitrary code exec outside of the sandbox
poc.patch
7.5 KB Download
Project Member Comment 6 by bugdroid1@chromium.org, Jul 12
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a86e8381e9d24ce160db8ceff5fe5e396c970eb6

commit a86e8381e9d24ce160db8ceff5fe5e396c970eb6
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Jul 12 02:33:32 2017

Service Manager: Fix client_process capability enforcement

Changes Service Manager to actually enforce the client_process
capability when handling StartServiceWithProcess requests from
service instances. This enforcement regressed in r444496.

BUG= 740710 
R=jcivelli@chromium.org

Change-Id: I6f0e6880f5c48a13d1b769ae6903ef96746f9030
Reviewed-on: https://chromium-review.googlesource.com/566479
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485809}
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/ios/web/public/app/mojo/web_browser_manifest.json
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/ios/web/public/app/mojo/web_packaged_services_manifest.json
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/services/service_manager/service_manager.cc
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/services/service_manager/tests/service_manager/service_manager_unittest.cc
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/services/service_manager/tests/service_manager/service_manager_unittest_manifest.json
[modify] https://crrev.com/a86e8381e9d24ce160db8ceff5fe5e396c970eb6/services/service_manager/tests/service_manager/target_manifest.json

Labels: Merge-Request-60
Status: Fixed
Requesting merge to 60. It's my understanding that there won't be another 59 release, yes?
Project Member Comment 8 by sheriffbot@chromium.org, Jul 12
Labels: -Merge-Request-60 Hotlist-Merge-Review Merge-Review-60
This bug requires manual review: We are only 12 days from stable.
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), josafat@(ChromeOS), bustamante@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 9 by sheriffbot@chromium.org, Jul 12
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -Merge-Review-60 Merge-Approved-60
Approving merge to M60. Yes, there are no plans for M59 release since M60 is 2 weeks away. 
Project Member Comment 11 by bugdroid1@chromium.org, Jul 12
Labels: -merge-approved-60 merge-merged-3112
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/491896edd626cdcb76a9ff6f490b48cd542978de

commit 491896edd626cdcb76a9ff6f490b48cd542978de
Author: Ken Rockot <rockot@chromium.org>
Date: Wed Jul 12 17:28:05 2017

Service Manager: Fix client_process capability enforcement

Changes Service Manager to actually enforce the client_process
capability when handling StartServiceWithProcess requests from
service instances. This enforcement regressed in r444496.

BUG= 740710 
R=jcivelli@chromium.org
TBR=rockot@chromium.org

(cherry picked from commit a86e8381e9d24ce160db8ceff5fe5e396c970eb6)

Change-Id: I6f0e6880f5c48a13d1b769ae6903ef96746f9030
Reviewed-on: https://chromium-review.googlesource.com/566479
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Jay Civelli <jcivelli@chromium.org>
Commit-Queue: Ken Rockot <rockot@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#485809}
Reviewed-on: https://chromium-review.googlesource.com/568593
Reviewed-by: Ken Rockot <rockot@chromium.org>
Cr-Commit-Position: refs/branch-heads/3112@{#595}
Cr-Branched-From: b6460e24cf59f429d69de255538d0fc7a425ccf9-refs/heads/master@{#474897}
[modify] https://crrev.com/491896edd626cdcb76a9ff6f490b48cd542978de/services/service_manager/service_manager.cc
[modify] https://crrev.com/491896edd626cdcb76a9ff6f490b48cd542978de/services/service_manager/tests/service_manager/service_manager_unittest.cc
[modify] https://crrev.com/491896edd626cdcb76a9ff6f490b48cd542978de/services/service_manager/tests/service_manager/service_manager_unittest_manifest.json
[modify] https://crrev.com/491896edd626cdcb76a9ff6f490b48cd542978de/services/service_manager/tests/service_manager/target_manifest.json

Labels: -M-59 M-61 M-60
Labels: Release-0-M60
Labels: -ReleaseBlock-Stable
Project Member Comment 15 by sheriffbot@chromium.org, Oct 18
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
Components: -Internals>ServiceManager Internals>Services>ServiceManager
Sign in to add a comment