Skip to content

Commit

Permalink
Don't create our own temporary mount point for pivot_root
Browse files Browse the repository at this point in the history
An attacker could pre-create /tmp/.bubblewrap-$UID and make it a
non-directory, non-symlink (in which case mounting our tmpfs would fail,
causing denial of service), or make it a symlink under their control
(potentially allowing bad things if the protected_symlinks sysctl is
not enabled).

Instead, temporarily mount the tmpfs on a directory that we are sure
exists and is not attacker-controlled. /tmp (the directory itself, not
a subdirectory) will do.

Fixes: #304
Bug-Debian: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=923557
Signed-off-by: Simon McVittie <smcv@debian.org>

Closes: #305
Approved by: cgwalters
  • Loading branch information
smcv authored and rh-atomic-bot committed Mar 5, 2019
1 parent 1622673 commit a66b227
Showing 1 changed file with 9 additions and 11 deletions.
20 changes: 9 additions & 11 deletions bubblewrap.c
Expand Up @@ -2046,7 +2046,7 @@ main (int argc,
char **argv)
{
mode_t old_umask;
cleanup_free char *base_path = NULL;
const char *base_path = NULL;
int clone_flags;
char *old_cwd = NULL;
pid_t pid;
Expand Down Expand Up @@ -2187,15 +2187,12 @@ main (int argc,
die_with_error ("Can't open /proc");

/* We need *some* mountpoint where we can mount the root tmpfs.
We first try in /run, and if that fails, try in /tmp. */
base_path = xasprintf ("/run/user/%d/.bubblewrap", real_uid);
if (ensure_dir (base_path, 0755))
{
free (base_path);
base_path = xasprintf ("/tmp/.bubblewrap-%d", real_uid);
if (ensure_dir (base_path, 0755))
die_with_error ("Creating root mountpoint failed");
}
* Because we use pivot_root, it won't appear to be mounted from
* the perspective of the sandboxed process, so we can use anywhere
* that is sure to exist, that is sure to not be a symlink controlled
* by someone malicious, and that we won't immediately need to
* access ourselves. */
base_path = "/tmp";

__debug__ (("creating new namespace\n"));

Expand Down Expand Up @@ -2400,7 +2397,8 @@ main (int argc,
/* We create a subdir "$base_path/newroot" for the new root, that
* way we can pivot_root to base_path, and put the old root at
* "$base_path/oldroot". This avoids problems accessing the oldroot
* dir if the user requested to bind mount something over / */
* dir if the user requested to bind mount something over / (or
* over /tmp, now that we use that for base_path). */

if (mkdir ("newroot", 0755))
die_with_error ("Creating newroot failed");
Expand Down

0 comments on commit a66b227

Please sign in to comment.