minijail: Check for symlinks immediately before mount().
Checking for symlinks at minijail_bind() time still leaves a pretty big
window for the check to be raced and the source path of a bind mount to
be modified between minijail_bind() and the corresponding mount() call.
Check immediately before the mount() call to make the window as small
as possible.
BUG=b:243161816
TEST=security.Minijail*, login.Chrome, arc.Boot.
Change-Id: I232274afb650df217ff94db647e535593b80fbdd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/minijail/+/3841217
Auto-Submit: Jorge Lucangeli Obes <jorgelo@chromium.org>
Reviewed-by: Ben Scarlato <akhna@google.com>
Commit-Queue: Jorge Lucangeli Obes <jorgelo@chromium.org>
Tested-by: Jorge Lucangeli Obes <jorgelo@chromium.org>
diff --git a/libminijail.c b/libminijail.c
index 794a896..150b0c2 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -996,12 +996,20 @@
{
unsigned long flags = MS_BIND;
+ /*
+ * Check for symlinks in bind-mount source paths to warn the user early.
+ * Minijail will perform one final check immediately before the mount()
+ * call.
+ */
if (!is_valid_bind_path(src)) {
warn("src '%s' is not a valid bind mount path", src);
return -ELOOP;
}
- /* |dest| might not yet exist. */
+ /*
+ * Symlinks in |dest| are blocked by the ChromiumOS LSM:
+ * <kernel>/security/chromiumos/lsm.c#77
+ */
if (!writeable)
flags |= MS_RDONLY;
@@ -1870,6 +1878,19 @@
}
}
+ /*
+ * Do a final check for symlinks in |m->src|.
+ * |m->src| will only contain a valid path when purely bind-mounting
+ * (but not when remounting a bind mount).
+ *
+ * Short of having a version of mount(2) that can take fd's, this is the
+ * smallest we can make the TOCTOU window.
+ */
+ if (has_bind_flag && !has_remount_flag && !is_valid_bind_path(m->src)) {
+ warn("src '%s' is not a valid bind mount path", m->src);
+ goto error;
+ }
+
ret = mount(m->src, dest, m->type, m->flags, m->data);
if (ret) {
pwarn("cannot mount '%s' as '%s' with flags %#lx", m->src, dest,
