Do not leak outside root dir fd into the child.
Also adds O_CLOEXEC to all open calls to be on the safe side. In the
future, we should look into doing some sanity checks before execve like
Chromium's sandbox does:
https://code.google.com/p/chromium/codesearch#chromium/src/sandbox/linux/services/credentials.cc&l=320
If we want to further prevent people from shooting themselves in the
foot, we could also check that no fds are open, except for duping
/dev/null over 0, 1, and 2.
TEST=Built and tested that an fd to / is not leaked.
Bug: None
Change-Id: I41993a6aec9ce48bd34d191c3949f313ba80ca73
diff --git a/libminijail.c b/libminijail.c
index 1d90aef..bcbaed5 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -390,7 +390,7 @@
void API minijail_namespace_enter_vfs(struct minijail *j, const char *ns_path)
{
- int ns_fd = open(ns_path, O_RDONLY);
+ int ns_fd = open(ns_path, O_RDONLY | O_CLOEXEC);
if (ns_fd < 0) {
pdie("failed to open namespace '%s'", ns_path);
}
@@ -418,7 +418,7 @@
void API minijail_namespace_enter_net(struct minijail *j, const char *ns_path)
{
- int ns_fd = open(ns_path, O_RDONLY);
+ int ns_fd = open(ns_path, O_RDONLY | O_CLOEXEC);
if (ns_fd < 0) {
pdie("failed to open namespace '%s'", ns_path);
}
@@ -961,7 +961,7 @@
ret = snprintf(fname, sz, "/proc/%d/uid_map", j->initpid);
if (ret < 0 || (size_t)ret >= sz)
die("failed to write file name of uid_map");
- fd = open(fname, O_WRONLY);
+ fd = open(fname, O_WRONLY | O_CLOEXEC);
if (fd < 0)
pdie("failed to open '%s'", fname);
len = strlen(j->uidmap);
@@ -973,7 +973,7 @@
ret = snprintf(fname, sz, "/proc/%d/gid_map", j->initpid);
if (ret < 0 || (size_t)ret >= sz)
die("failed to write file name of gid_map");
- fd = open(fname, O_WRONLY);
+ fd = open(fname, O_WRONLY | O_CLOEXEC);
if (fd < 0)
pdie("failed to open '%s'", fname);
len = strlen(j->gidmap);
@@ -1085,10 +1085,10 @@
* Keep the fd for both old and new root.
* It will be used in fchdir later.
*/
- oldroot = open("/", O_DIRECTORY | O_RDONLY);
+ oldroot = open("/", O_DIRECTORY | O_RDONLY | O_CLOEXEC);
if (oldroot < 0)
pdie("failed to open / for fchdir");
- newroot = open(j->chrootdir, O_DIRECTORY | O_RDONLY);
+ newroot = open(j->chrootdir, O_DIRECTORY | O_RDONLY | O_CLOEXEC);
if (newroot < 0)
pdie("failed to open %s for fchdir", j->chrootdir);
@@ -1115,6 +1115,10 @@
/* Change back to the new root. */
if (fchdir(newroot))
return -errno;
+ if (close(oldroot))
+ return -errno;
+ if (close(newroot))
+ return -errno;
if (chroot("/"))
return -errno;
/* Set correct CWD for getcwd(3). */
