Issue metadata
Sign in to add a comment
|
Security: arc setup code in session_manager writes lots of untrusted file system locations carelessly |
||||||||||||||||||||||
Issue descriptionSpecifically, InstallDirectory and WriteToFile are susceptible to attacks where the target locations already contain a file system object, such as symlink, fifo, etc. placed by an attacker. The code will happily send its writes to an attacker-controlled file system location then. It doesn't help of course that session_manager still runs as root, but that's a separate problem. We should make the file creation code safe against this class of attacks. Marking as Severity-low for now since there is no known exploit. Does anyone on the ARC++ team have cycles to run with this?
,
Jan 30 2018
Is anyone still planning to look at this?
,
Jan 31 2018
hmm in the meantime the implementation moved to run_oci.
,
Jan 31 2018
Does run_oci have the same problem? Does run_oci write files without checking as well?
,
Jan 31 2018
run_oci/libcontainer writes to cgroups. We can add a O_NOFOLLOW there since we're using open(2) already. They also write the pid file courtesy of minijail (which uses fopen, so it might follow symlinks too). arc_setup is the main contributor to that, but it does have a wrapper function that we can switch to use O_NOFOLLOW and gift the file descriptor to a base::File instead of letting base::File handle it for us.
,
Jan 31 2018
Sorry I didn't really have time to take a look at this. Junichi: I think the code Mattias is referring to is platform2/arc/setup/*.cc, not platform2/login_manager/ because the latter doesn't really write/install files.
,
Jan 31 2018
,
Jan 31 2018
,
Feb 1 2018
CLs for run_oci (by Luis) and arc_setup (by me) are under review. BTW, WriteToFile() in aosp/external/libbrillo/brillo/file_utils.cc might have the same issue. It only uses base:: functions, and does not seem to check symlinks/fifos.
,
Feb 2 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/fd81db4df50473408ca086cdbd4d887928548318 commit fd81db4df50473408ca086cdbd4d887928548318 Author: Luis Hector Chavez <lhchavez@google.com> Date: Fri Feb 02 05:29:40 2018 libcontainer: Make OpenCgroupFile be more careful This change makes OpenCgroupFile use O_NOFOLLOW to open files in order to be more careful and avoid being tricked into writing stuff into unintended locations. It also calls fstat(2) after the file has been opened to ensure it does not write to somewhere that is not a file (e.g. a pipe). BUG= chromium:767018 TEST=FEATURES="test" emerge-${BOARD} chromeos-base/libcontainer Change-Id: Ia77698e863a163cf1eff95552f065de7fba5df21 Reviewed-on: https://chromium-review.googlesource.com/896182 Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com> Tested-by: Luis Hector Chavez <lhchavez@chromium.org> Reviewed-by: Luis Hector Chavez <lhchavez@chromium.org> Reviewed-by: Mattias Nissler <mnissler@chromium.org> [modify] https://crrev.com/fd81db4df50473408ca086cdbd4d887928548318/libcontainer/cgroup.cc [modify] https://crrev.com/fd81db4df50473408ca086cdbd4d887928548318/libcontainer/cgroup_unittest.cc
,
Feb 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform2/+/7ae1b856e01bb04e462a1a839ffcdf0d9f87a557 commit 7ae1b856e01bb04e462a1a839ffcdf0d9f87a557 Author: yusukes <yusukes@google.com> Date: Tue Feb 06 03:09:22 2018 arc-setup: Improve WriteToFile() and MkdirRecursively() With this CL, these functions check if the path is safe to open and bail out when it isn't. BUG= chromium:767018 TEST=FEATURES="test" emerge-caroline-arcnext arc-setup TEST=ARC container still starts Change-Id: I9a50be069c21baf97ab9cee4ecedcba831c4dd2f Reviewed-on: https://chromium-review.googlesource.com/896371 Commit-Ready: Yusuke Sato <yusukes@chromium.org> Tested-by: Yusuke Sato <yusukes@chromium.org> Reviewed-by: Mattias Nissler <mnissler@chromium.org> [modify] https://crrev.com/7ae1b856e01bb04e462a1a839ffcdf0d9f87a557/arc/setup/arc_setup_util.cc [modify] https://crrev.com/7ae1b856e01bb04e462a1a839ffcdf0d9f87a557/arc/setup/arc_setup_util_unittest.cc
,
Feb 6 2018
,
Feb 8 2018
,
May 15 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
,
Sep 19
|
|||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||
Comment 1 by yusukes@chromium.org
, Sep 21 2017Status: Assigned (was: Available)