summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorAlexander Larsson <alexl@redhat.com>2020-03-26 15:36:44 +0100
committerAlexander Larsson <alexl@redhat.com>2020-03-26 15:36:44 +0100
commit5404a15d34301a5a0dd5930203e03c76b80ebf21 (patch)
treeb45ed4b91e5ce7fec28dfd918a91519f00cf4d09
parent3ace81ca071bf3322dc7fefb140389b18f70fc59 (diff)
downloadbubblewrap-5404a15d34301a5a0dd5930203e03c76b80ebf21.tar.gz
Don't rely on geteuid() to know when to switch back from setuid root
As pointed out by Stephen Röttger <sroettger@google.com>, in drop_privs() we only drop root in the setuid case if geteuid() is 0. Typically geteuid() == 0 means we were setuid root and have not yet switched away from it. However, it is possible to make the geteuid call fail by passing a --userns2 namespace which doesn't have 0 mapped (i.e. where geteuid() will return the owerflow uid instead). If you do this, the pid 1 process in the sandbox will continue running as host uid 0, while dropping the dumpable flag, and at this point the user can ptrace attach the process and have root permissions. We fix this by not relying on the geteuid() call to know when we need to drop root uid, but rather keep track of whether we already switched from it.
-rw-r--r--bubblewrap.c12
1 files changed, 7 insertions, 5 deletions
diff --git a/bubblewrap.c b/bubblewrap.c
index 3b6b645..b3b501f 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -837,11 +837,13 @@ switch_to_user_with_privs (void)
/* Call setuid() and use capset() to adjust capabilities */
static void
-drop_privs (bool keep_requested_caps)
+drop_privs (bool keep_requested_caps,
+ bool changed_uid)
{
assert (!keep_requested_caps || !is_privileged);
/* Drop root uid */
- if (geteuid () == 0 && setuid (opt_sandbox_uid) < 0)
+ if (is_privileged && !changed_uid &&
+ setuid (opt_sandbox_uid) < 0)
die_with_error ("unable to drop root uid");
drop_all_caps (keep_requested_caps);
@@ -2502,7 +2504,7 @@ main (int argc,
die_with_error ("Setting userns2 failed");
/* We don't need any privileges in the launcher, drop them immediately. */
- drop_privs (FALSE);
+ drop_privs (FALSE, FALSE);
/* Optionally bind our lifecycle to that of the parent */
handle_die_with_parent ();
@@ -2677,7 +2679,7 @@ main (int argc,
if (child == 0)
{
/* Unprivileged setup process */
- drop_privs (FALSE);
+ drop_privs (FALSE, TRUE);
close (privsep_sockets[0]);
setup_newroot (opt_unshare_pid, privsep_sockets[1]);
exit (0);
@@ -2775,7 +2777,7 @@ main (int argc,
}
/* All privileged ops are done now, so drop caps we don't need */
- drop_privs (!is_privileged);
+ drop_privs (!is_privileged, TRUE);
if (opt_block_fd != -1)
{