diff --git a/bubblewrap.c b/bubblewrap.c
index 3b9719c4..bccd8d8a 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -1449,6 +1449,20 @@ warn_only_last_option (const char *name)
warn ("Only the last %s option will take effect", name);
}
+/* Path arguments are turned into real filenames by concatenating them onto
+ "/oldroot/" or "/newroot/", so an empty path would silently be taken to
+ mean the root directory. That is never what the caller meant, so treat it
+ as a usage error. */
+static const char *
+path_argument (const char *option,
+ const char *path)
+{
+ if (path[0] == '\0')
+ die ("%s does not take an empty path argument", option);
+
+ return path;
+}
+
static void
make_setup_overlay_src_ops (const char *const *const argv)
{
@@ -1470,7 +1484,7 @@ make_setup_overlay_src_ops (const char *const *const argv)
for (i = 1; i <= next_overlay_src_count; i++)
{
op = setup_op_new (SETUP_OVERLAY_SRC);
- op->source = argv[1 - 2 * i];
+ op->source = path_argument ("--overlay-src", argv[1 - 2 * i]);
}
next_overlay_src_count = 0;
}
@@ -1648,7 +1662,7 @@ parse_args_recurse (int *argcp,
if (opt_chdir_path != NULL)
warn_only_last_option ("--chdir");
- opt_chdir_path = argv[1];
+ opt_chdir_path = path_argument (arg, argv[1]);
argv++;
argc--;
}
@@ -1666,7 +1680,7 @@ parse_args_recurse (int *argcp,
die ("--remount-ro takes one argument");
op = setup_op_new (SETUP_REMOUNT_RO_NO_RECURSIVE);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
argv++;
argc--;
@@ -1678,8 +1692,8 @@ parse_args_recurse (int *argcp,
die ("%s takes two arguments", arg);
op = setup_op_new (SETUP_BIND_MOUNT);
- op->source = argv[1];
- op->dest = argv[2];
+ op->source = path_argument (arg, argv[1]);
+ op->dest = path_argument (arg, argv[2]);
if (strcmp(arg, "--bind-try") == 0)
op->flags = ALLOW_NOTEXIST;
@@ -1693,8 +1707,8 @@ parse_args_recurse (int *argcp,
die ("%s takes two arguments", arg);
op = setup_op_new (SETUP_RO_BIND_MOUNT);
- op->source = argv[1];
- op->dest = argv[2];
+ op->source = path_argument (arg, argv[1]);
+ op->dest = path_argument (arg, argv[2]);
if (strcmp(arg, "--ro-bind-try") == 0)
op->flags = ALLOW_NOTEXIST;
@@ -1708,8 +1722,8 @@ parse_args_recurse (int *argcp,
die ("%s takes two arguments", arg);
op = setup_op_new (SETUP_DEV_BIND_MOUNT);
- op->source = argv[1];
- op->dest = argv[2];
+ op->source = path_argument (arg, argv[1]);
+ op->dest = path_argument (arg, argv[2]);
if (strcmp(arg, "--dev-bind-try") == 0)
op->flags = ALLOW_NOTEXIST;
@@ -1735,7 +1749,7 @@ parse_args_recurse (int *argcp,
op = setup_op_new (SETUP_BIND_MOUNT);
op->source = xasprintf ("/proc/self/fd/%d", src_fd);
op->fd = src_fd;
- op->dest = argv[2];
+ op->dest = path_argument (arg, argv[2]);
argv += 2;
argc -= 2;
@@ -1758,10 +1772,10 @@ parse_args_recurse (int *argcp,
die ("--overlay requires at least one --overlay-src");
op = setup_op_new (SETUP_OVERLAY_MOUNT);
- op->source = argv[1];
+ op->source = path_argument (arg, argv[1]);
workdir_op = setup_op_new (SETUP_OVERLAY_SRC);
- workdir_op->source = argv[2];
- op->dest = argv[3];
+ workdir_op->source = path_argument (arg, argv[2]);
+ op->dest = path_argument (arg, argv[3]);
make_setup_overlay_src_ops (argv);
argv += 3;
@@ -1776,7 +1790,7 @@ parse_args_recurse (int *argcp,
die ("--tmp-overlay requires at least one --overlay-src");
op = setup_op_new (SETUP_TMP_OVERLAY_MOUNT);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
make_setup_overlay_src_ops (argv);
opt_tmp_overlay_count++;
@@ -1792,7 +1806,7 @@ parse_args_recurse (int *argcp,
die ("--ro-overlay requires at least two --overlay-src");
op = setup_op_new (SETUP_RO_OVERLAY_MOUNT);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
make_setup_overlay_src_ops (argv);
argv += 1;
@@ -1804,7 +1818,7 @@ parse_args_recurse (int *argcp,
die ("--proc takes an argument");
op = setup_op_new (SETUP_MOUNT_PROC);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
argv += 1;
argc -= 1;
@@ -1845,7 +1859,7 @@ parse_args_recurse (int *argcp,
die ("--dev takes an argument");
op = setup_op_new (SETUP_MOUNT_DEV);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
opt_needs_devpts = true;
argv += 1;
@@ -1857,7 +1871,7 @@ parse_args_recurse (int *argcp,
die ("--tmpfs takes an argument");
op = setup_op_new (SETUP_MOUNT_TMPFS);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
/* We historically hard-coded the mode of a tmpfs as 0755. */
if (next_perms >= 0)
@@ -1882,7 +1896,7 @@ parse_args_recurse (int *argcp,
die ("--mqueue takes an argument");
op = setup_op_new (SETUP_MOUNT_MQUEUE);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
argv += 1;
argc -= 1;
@@ -1893,7 +1907,7 @@ parse_args_recurse (int *argcp,
die ("--dir takes an argument");
op = setup_op_new (SETUP_MAKE_DIR);
- op->dest = argv[1];
+ op->dest = path_argument (arg, argv[1]);
/* We historically hard-coded the mode of a --dir as 0755. */
if (next_perms >= 0)
@@ -1919,7 +1933,7 @@ parse_args_recurse (int *argcp,
op = setup_op_new (SETUP_MAKE_FILE);
op->fd = file_fd;
- op->dest = argv[2];
+ op->dest = path_argument (arg, argv[2]);
/* We historically hard-coded the mode of a --file as 0666. */
if (next_perms >= 0)
@@ -1945,7 +1959,7 @@ parse_args_recurse (int *argcp,
op = setup_op_new (SETUP_MAKE_BIND_FILE);
op->fd = file_fd;
- op->dest = argv[2];
+ op->dest = path_argument (arg, argv[2]);
/* This is consistent with previous bubblewrap behaviour:
* before implementing --perms, we took the permissions
@@ -1973,7 +1987,7 @@ parse_args_recurse (int *argcp,
op = setup_op_new (SETUP_MAKE_RO_BIND_FILE);
op->fd = file_fd;
- op->dest = argv[2];
+ op->dest = path_argument (arg, argv[2]);
/* This is consistent with previous bubblewrap behaviour:
* before implementing --perms, we took the permissions
@@ -1993,8 +2007,8 @@ parse_args_recurse (int *argcp,
die ("--symlink takes two arguments");
op = setup_op_new (SETUP_MAKE_SYMLINK);
- op->source = argv[1];
- op->dest = argv[2];
+ op->source = path_argument (arg, argv[1]);
+ op->dest = path_argument (arg, argv[2]);
argv += 2;
argc -= 2;
@@ -2004,7 +2018,7 @@ parse_args_recurse (int *argcp,
if (argc < 2)
die ("--lock-file takes an argument");
- (void) lock_file_new (argv[1]);
+ (void) lock_file_new (path_argument (arg, argv[1]));
argv += 1;
argc -= 1;
@@ -2434,7 +2448,7 @@ parse_args_recurse (int *argcp,
op = setup_op_new (SETUP_CHMOD);
op->flags = NO_CREATE_DEST;
op->perms = (int) perms;
- op->dest = argv[2];
+ op->dest = path_argument (arg, argv[2]);
argv += 2;
argc -= 2;
diff --git a/bwrap.xml b/bwrap.xml
index ca717abd..b48cf5b4 100644
--- a/bwrap.xml
+++ b/bwrap.xml
@@ -73,6 +73,10 @@
When options are used multiple times, the last option wins, unless otherwise
specified.
+
+ An empty string is not a valid path, so options that take a path
+ reject it with an error.
+
General options:
diff --git a/tests/test-run.sh b/tests/test-run.sh
index 2d0d307d..a78e2b56 100755
--- a/tests/test-run.sh
+++ b/tests/test-run.sh
@@ -176,6 +176,34 @@ fi
assert_file_has_content err.txt "^bwrap: Can't find source path.*source-enoent"
ok "error prefixing"
+# Test that an empty path argument is rejected. Empty paths used to be
+# interpreted as the root directory, so for example --bind-try "" DEST
+# silently gave the sandbox access to the whole filesystem.
+for opt in --bind --bind-try --dev-bind --dev-bind-try --ro-bind --ro-bind-try; do
+ if $RUN "$opt" "" /mnt true 2>err.txt; then
+ assert_not_reached "$opt accepted an empty source"
+ fi
+ assert_file_has_content err.txt "^bwrap: $opt does not take an empty path argument"
+
+ if $RUN "$opt" / "" true 2>err.txt; then
+ assert_not_reached "$opt accepted an empty destination"
+ fi
+ assert_file_has_content err.txt "^bwrap: $opt does not take an empty path argument"
+done
+
+for opt in --chdir --dev --dir --mqueue --proc --remount-ro --tmpfs; do
+ if $RUN "$opt" "" true 2>err.txt; then
+ assert_not_reached "$opt accepted an empty path"
+ fi
+ assert_file_has_content err.txt "^bwrap: $opt does not take an empty path argument"
+done
+
+if $RUN --symlink /usr "" true 2>err.txt; then
+ assert_not_reached "--symlink accepted an empty destination"
+fi
+assert_file_has_content err.txt "^bwrap: --symlink does not take an empty path argument"
+ok "empty path arguments are rejected"
+
if ! ${is_uidzero}; then
# When invoked as non-root, check that by default we have no caps left
for OPT in "" "--unshare-user-try --as-pid-1" "--unshare-user-try" "--as-pid-1"; do