New issue
Advanced search Search tips

Issue 757577 link

Starred by 3 users
Status: Available
Owner: ----
Cc:
kerrnel@chromium.org
rsesek@chromium.org
mark@chromium.org
wfh@chromium.org
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Mac
Pri: 3
Type: Bug



Sign in to add a comment

Remove sandbox dependency on //base

Project Member Reported by kerrnel@chromium.org, Aug 21 2017 
The Google Chrome Helper executable currently links against many frameworks that it should not be linked against. Each of these frameworks brings initialization code that may increase attack surface.

Robert pointed out that if sandbox were to no longer use //base, almost all of these Frameworks should go away.

We can do this through the following:

1) No base::ScopedFD in //sandbox, just release FDs in destructors
2) Write sandbox_logging.h to provide basic logging
3) Write a 'verify_order' like script to verify the deps of //sandbox going forward

$ otool -L out/Official/Google\ Chrome.app/Contents/Versions/62.0.3190.0/Google\ Chrome\ Helper.app/Contents/MacOS/Google\ Chrome\ Helper 
out/Official/Google Chrome.app/Contents/Versions/62.0.3190.0/Google Chrome Helper.app/Contents/MacOS/Google Chrome Helper:
	/usr/lib/libsandbox.1.dylib (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 48.0.0)
	/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 1504.82.104)
	/usr/lib/libbsm.0.dylib (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 1349.64.0)
	/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
	/System/Library/Frameworks/Security.framework/Versions/A/Security (compatibility version 1.0.0, current version 57740.51.2)
	/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 307.5.0)
	/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1238.50.2)
	/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1070.22.0)
	/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 775.19.0)
	/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 1.0.0)
	/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 1349.63.0)
	/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)

Comment 1 by rsesek@chromium.org, Sep 1 2017

Components: Internals>Sandbox
Labels: OS-Android OS-Linux OS-Mac
I'd also like to trim //base from the Seccomp-BPF part of the sandbox as well, since it'll make it easier to load just the libsandbox.so on Android.
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 4 2017 

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

commit b35515dfbe21f74c4c48a65a3630337219fd490b
Author: Greg Kerr <kerrnel@chromium.org>
Date: Wed Oct 04 17:56:39 2017

Remove sandbox/mac dependency on //base.

This removes the mac sandbox libraries dependency on //base. 
The dependency was causing the Chromium Helper binary to be 
linked against everything //base uses 
(AppKit, Security.framework, etc.), which is bad for security 
because of the resource access that happens in those libraries 
initializers.

Bug: 757577
Change-Id: I4699b9cd490c188e1659932e5c97621e37438f32
Reviewed-on: https://chromium-review.googlesource.com/676262
Commit-Queue: Greg Kerr <kerrnel@chromium.org>
Reviewed-by: Mark Mentovai <mark@chromium.org>
Reviewed-by: Robert Sesek <rsesek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506440}
[modify] https://crrev.com/b35515dfbe21f74c4c48a65a3630337219fd490b/sandbox/mac/BUILD.gn
[add] https://crrev.com/b35515dfbe21f74c4c48a65a3630337219fd490b/sandbox/mac/sandbox_logging.cc
[add] https://crrev.com/b35515dfbe21f74c4c48a65a3630337219fd490b/sandbox/mac/sandbox_logging.h
[modify] https://crrev.com/b35515dfbe21f74c4c48a65a3630337219fd490b/sandbox/mac/seatbelt_exec.cc
[modify] https://crrev.com/b35515dfbe21f74c4c48a65a3630337219fd490b/sandbox/mac/seatbelt_exec.h

Comment 3 by kerrnel@chromium.org, Oct 4 2017

Status: Fixed (was: Assigned)

Comment 4 by rsesek@chromium.org, Oct 4 2017

Cc: kerrnel@chromium.org
Owner: ----
Status: Available (was: Fixed)
Going to keep this open for the other platform work.
Project Member

Comment 5 by sheriffbot@chromium.org, Oct 5

Labels: Hotlist-Recharge-Cold
Status: Untriaged (was: Available)
This issue has been Available for over a year. If it's no longer important or seems unlikely to be fixed, please consider closing it out. If it is important, please re-triage the issue.

Sorry for the inconvenience if the bug really should have been left as Available.

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

Comment 6 by rsesek@chromium.org, Oct 8

Status: Available (was: Untriaged)
►

Sign in to add a comment