Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 131 additions & 0 deletions SPECS/bubblewrap/CVE-2020-5291.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
From 5404a15d34301a5a0dd5930203e03c76b80ebf21 Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Thu, 26 Mar 2020 15:36:44 +0100
Subject: [PATCH 1/3] Don't rely on geteuid() to know when to switch back from
setuid root
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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.
---
bubblewrap.c | 12 +++++++-----
1 file 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)
{

From 61955e933dfb5811377dbae3dd40eec60853f37a Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Fri, 27 Mar 2020 08:28:26 +0100
Subject: [PATCH 2/3] Don't support --userns2 in setuid mode

We already don't support --userns, so supporting --userns2 seems
like asking for problems with no upside.
---
bubblewrap.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/bubblewrap.c b/bubblewrap.c
index b3b501f..0db9f2c 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -2301,6 +2301,9 @@ main (int argc,
if (opt_userns_fd != -1 && is_privileged)
die ("--userns doesn't work in setuid mode");

+ if (opt_userns2_fd != -1 && is_privileged)
+ die ("--userns2 doesn't work in setuid mode");
+
/* We have to do this if we weren't installed setuid (and we're not
* root), so let's just DWIM */
if (!is_privileged && getuid () != 0 && opt_userns_fd == -1)

From 6f815ceeadd2728903b83f227945769c582e5c74 Mon Sep 17 00:00:00 2001
From: Alexander Larsson <alexl@redhat.com>
Date: Mon, 30 Mar 2020 12:31:36 +0200
Subject: [PATCH 3/3] drop_privs: More explicit argument name

changed_uid => already_changed_uid
---
bubblewrap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/bubblewrap.c b/bubblewrap.c
index 0db9f2c..b3d52bc 100644
--- a/bubblewrap.c
+++ b/bubblewrap.c
@@ -838,11 +838,11 @@ switch_to_user_with_privs (void)
/* Call setuid() and use capset() to adjust capabilities */
static void
drop_privs (bool keep_requested_caps,
- bool changed_uid)
+ bool already_changed_uid)
{
assert (!keep_requested_caps || !is_privileged);
/* Drop root uid */
- if (is_privileged && !changed_uid &&
+ if (is_privileged && !already_changed_uid &&
setuid (opt_sandbox_uid) < 0)
die_with_error ("unable to drop root uid");

6 changes: 5 additions & 1 deletion SPECS/bubblewrap/bubblewrap.spec
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
Summary: setuid implementation of a subset of user namespaces.
Name: bubblewrap
Version: 0.3.0
Release: 5%{?dist}
Release: 6%{?dist}
License: LGPLv2+
URL: https://github.com/containers/bubblewrap/
Group: Applications/System
Expand All @@ -11,6 +11,7 @@ Source0: https://github.com/containers/bubblewrap/releases/download/v%{ve
# This vulnerability only applies to version >= 0.4.0. Ignore the warnings against it.
Patch0: CVE-2020-5291.nopatch
Patch1: CVE-2019-12439.patch
Patch2: CVE-2020-5291.patch
BuildRequires: autoconf
BuildRequires: automake
BuildRequires: libtool
Expand All @@ -24,6 +25,7 @@ The original bubblewrap code existed before user namespaces - it inherits code f
%prep
%setup -q
%patch1 -p1
%patch2 -p1
%build

./configure \
Expand Down Expand Up @@ -51,6 +53,8 @@ rm -rf %{buildroot}/*
%{_datadir}/bash-completion/completions/bwrap

%changelog
* Thu Feb 18 2021 Mariner Autopatcher <cblmargh@microsoft.com> 0.3.0-6
- Added patch files ./patches/CVE-2020-5291/CVE-2020-5291.patch
* Thu May 21 2020 Ruying Chen <v-ruyche@microsoft.com> - 0.3.0-5
- Fixed CVE-2019-12439

Expand Down