CVE-2019-12439: Improper Input Validation (CWE-20)
value: bubblewrap.c in Bubblewrap before 0.3.3 misuses temporary directories in /tmp as a mount point. In some particular configurations (related to XDG_RUNTIME_DIR), a local attacker may abuse this flaw to prevent other users from executing bubblewrap or potentially execute code.
CWE description: The product receives input or data, but it does
        not validate or incorrectly validates that the input has the
        properties that are required to process the data safely and
        correctly.
CVSS v3 base score: 7.8
Published: 2019-05-29T15:29Z
Repository: https://github.com/projectatomic/bubblewrap
Language: C
Files changed: bubblewrap.c
Commit date: 2019-03-06 13:41:29 +0000
Commit message: Don't create our own temporary mount point for pivot_root

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

Diff: diff --git a/bubblewrap.c b/bubblewrap.c
index a3fd61f..96cf2d5 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -2044,11 +2044,11 @@ read_overflowids (void)
 int
 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;
   int event_fd = -1;
   int child_wait_fd = -1;
@@ -2185,19 +2185,16 @@ main (int    argc,
   proc_fd = open ("/proc", O_PATH);
   if (proc_fd == -1)
     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"));
 
   if (opt_unshare_pid && !opt_as_pid_1)
     {
@@ -2398,11 +2395,12 @@ main (int    argc,
     die_with_error ("chdir base_path");
 
   /* 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");
 
   if (mount ("newroot", "newroot", NULL, MS_MGC_VAL | MS_BIND | MS_REC, NULL) < 0)

Vulnerable code:   cleanup_free char *base_path = NULL;
     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");
    }
   * dir if the user requested to bind mount something over / */
Fixed code:   const char *base_path = NULL;
   * 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";
   * dir if the user requested to bind mount something over / (or
   * over /tmp, now that we use that for base_path). */