Security: Sandbox escape via exposed "filesystem::mojom::Directory" mojo interface in "catalog" service |
|||||||||||||||||||||||||||||||||
Issue descriptionThe "catalog" mojo service is located in the browser process and allows callers to enumerate the currently registered service instances under the service manager. It also allows callers to filter services according to their exposed capabilities defined in their respective manifests. The service itself does not have manifest entries, and can therefore be bound from any mojo interface, including the "content_renderer" service. The service exposes two interfaces: "catalog::mojom::Catalog" (used to query the catalog status), and an instance of "filesystem::mojom::Directory". These interfaces are bound to the service's "service_manager::BinderRegistry" (by calling "AddInterface"), allowing external services to invoke "BindInterface" on their Connectors in order to bind to either interface remotely. I believe the latter interface, "filesystem::mojom::Directory", is accidentally exposed to external callers. The "Directory" interface (https://cs.chromium.org/chromium/src/components/filesystem/public/interfaces/directory.mojom) allows callers to arbitrarily read, write, delete, create and enumerate files under a given path. The "DirectoryImpl" class provides a concrete implementation of the "Directory" mojo interface, which facilitates the above operations. When the "catalog" service is initialised, it adds bindings for the two interfaces above (https://cs.chromium.org/chromium/src/services/catalog/catalog.cc?l=149). The "DirectoryImpl" instance bound to the "Directory" interface is then initialised using the following code: ... base::FilePath resources_path; base::PathService::Get(base::DIR_MODULE, &resources_path); mojo::MakeStrongBinding( std::make_unique<filesystem::DirectoryImpl>( resources_path, scoped_refptr<filesystem::SharedTempDir>(), thread_state->lock_table()), ... As we can see in the snippet above, the "base::DIR_MODULE" path is used as the root path for the exposed Directory instance. Consequently, the directory containing the "module" will be exposed to external callers, allowing them to modify any file under it. On Linux, this means the directory containing the Chromium binary itself is used as the root path -- allowing attackers to modify or even replace the chromium binary itself, as well as the shared libraries it loads (or other configuration files). Consequently, a compromised renderer can bind to the aforementioned instance in the "catalog" service and use it to modify the application's binary, resulting in a sandbox escape. I haven't checked other platforms yet, but I believe the same should apply for most platforms. VERSION Chromium 64.0.3282.0 64-bit Revision dd12859a9c856c6919cedf6c35d13b8b22af94e1-refs/heads/master@{#520743} OS Linux 4.4.0-97-generic REPRODUCTION CASE I'm attaching a small patch that adds code to the renderer process which binds to the aforementioned interface and creates an arbitrary file (named "arbitrary_file") under Chromium's base directory. Applying the patch and navigating to any page should result in the aforementioned file being created.
,
Dec 1 2017
,
Dec 1 2017
This used to be safe. Each mojo service used to have its binary placed in its own directory, containing only its own service, along with any files that it needed to read at runtime. (I assume that's what the Directory is for.) It appears there was a refactor (https://codereview.chromium.org/2645973006/) which then moved everything out of the directories.
,
Dec 1 2017
Ouch. Right, so because this is not used in production Chrome and it seems like quite a major issue, I am inclined to immediately break the Directory interface and then figure out what we need to do to fix mash services which depend on it.
,
Dec 1 2017
,
Dec 1 2017
,
Dec 1 2017
Going to make the capability inaccessible to most services first. Follow-up solution will be to get rid of the Directory interface altogether and instead add an explicit file access API to the service manager which will require manifest declarations to whitelist accessible files. +tsepez: sound reasonable?
,
Dec 1 2017
Thanks for investigating rockot! > Right, so because this is not used in production Chrome Could you clarify what you mean by this? Does this bug affect the Chrome binary? Please update the security impact and milestone labels accordingly.
,
Dec 1 2017
,
Dec 1 2017
,
Dec 1 2017
This absolutely affects the Chrome binary, what I mean is that no production code relies on the insecure interface being available. So disabling the interface will only break code which works behind a flag and/or with a special build config.
,
Dec 1 2017
,
Dec 1 2017
This would in principle affect Fuchsia too, right?
,
Dec 1 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dd8593661dae7acc659fb8d7ce0f1224dde31429 commit dd8593661dae7acc659fb8d7ce0f1224dde31429 Author: Ken Rockot <rockot@chromium.org> Date: Fri Dec 01 23:23:28 2017 Remove blanket module directory access from catalog service Several services require "app" capability from all other services (wildcard), which is in itself probably a Bad Thing; but it's especially bad now because the catalog service exposes module directory access through its app capability. This changes the catalog manifest to move the capability to a different more specific capability, and updates two of its consumers. May break other random mash/mushrome (i.e. non-production) services depend on the blanket directory access behavior since aura mus client library code depends on it in some environments. Bug: 791003 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel Change-Id: I81d81af8385fb1e013445a6b8c175ebf96388077 Reviewed-on: https://chromium-review.googlesource.com/804397 Commit-Queue: Ken Rockot <rockot@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Cr-Commit-Position: refs/heads/master@{#521115} [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/ash/manifest.json [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/mash/quick_launch/manifest.json [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/services/catalog/manifest.json [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/services/service_manager/service_manager.cc [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/services/ui/manifest.json [modify] https://crrev.com/dd8593661dae7acc659fb8d7ce0f1224dde31429/services/viz/manifest.json
,
Dec 1 2017
,
Dec 1 2017
Also filed bug 791186 to follow up with better (but less merge-friendly) changes.
,
Dec 1 2017
,
Dec 1 2017
This bug requires manual review: We are only 3 days from stable. Please contact the milestone owner if you have questions. Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Dec 2 2017
+awhalley@ for M63 merge review (Note: We're planning to cut M63 Stable RC soon @ 5:00 PM PT)
,
Dec 2 2017
Once this has had some bake time in dev/beta we can get this into 63 to be picked up if there are any future respins.
,
Dec 2 2017
,
Dec 7 2017
This also need a merge to M64, correct?
,
Dec 7 2017
Indeed, didn't realize it missed the cut.
,
Dec 8 2017
Your change meets the bar and is auto-approved for M64. Please go ahead and merge the CL to branch 3282 manually. Please contact 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
,
Dec 8 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c78fe161cd10e2f904482d196b66c8689320ef9 commit 1c78fe161cd10e2f904482d196b66c8689320ef9 Author: Ken Rockot <rockot@chromium.org> Date: Fri Dec 08 01:35:17 2017 Remove blanket module directory access from catalog service Several services require "app" capability from all other services (wildcard), which is in itself probably a Bad Thing; but it's especially bad now because the catalog service exposes module directory access through its app capability. This changes the catalog manifest to move the capability to a different more specific capability, and updates two of its consumers. May break other random mash/mushrome (i.e. non-production) services depend on the blanket directory access behavior since aura mus client library code depends on it in some environments. TBR=rockot@chromium.org (cherry picked from commit dd8593661dae7acc659fb8d7ce0f1224dde31429) Bug: 791003 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel Change-Id: I81d81af8385fb1e013445a6b8c175ebf96388077 Reviewed-on: https://chromium-review.googlesource.com/804397 Commit-Queue: Ken Rockot <rockot@chromium.org> Reviewed-by: Tom Sepez <tsepez@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#521115} Reviewed-on: https://chromium-review.googlesource.com/816355 Reviewed-by: Ken Rockot <rockot@chromium.org> Cr-Commit-Position: refs/branch-heads/3282@{#86} Cr-Branched-From: 5fdc0fab22ce7efd32532ee989b223fa12f8171e-refs/heads/master@{#520840} [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/ash/manifest.json [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/mash/quick_launch/manifest.json [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/services/catalog/manifest.json [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/services/service_manager/service_manager.cc [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/services/ui/manifest.json [modify] https://crrev.com/1c78fe161cd10e2f904482d196b66c8689320ef9/services/viz/manifest.json
,
Dec 13 2017
Ping for M63 merge review - this has been baking in M64 for a few days now.
,
Jan 8 2018
,
Jan 8 2018
M64 is only 2 weeks away, and no respins planned for M63.
,
Jan 22 2018
,
Jan 31 2018
Hello, Has a CVE been assigned for this issue? If not, would it be possible to assign one? Thanks! Gal.
,
Jan 31 2018
We don't assign for internally reported bugs as a matter of course, but no reason that we can't. This one's CVE-2018-6055 - thanks for the report!
,
Jan 31 2018
Thanks! I'll include the CVE in our tracker.
,
Mar 2 2018
,
Mar 10 2018
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
,
Mar 27 2018
,
Apr 25 2018
,
Oct 5
|
|||||||||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||||||||
Comment 1 by laginimaineb@google.com
, Dec 1 2017