Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

potentially insecure use of /tmp #304

Closed
smcv opened this issue Mar 2, 2019 · 9 comments

Comments

Projects
None yet
4 participants
@smcv
Copy link
Contributor

commented Mar 2, 2019

@jwilk reported this Debian bug (#923557):

[If] /run/user/<UID>/.bubblewrap/ doesn't exist and couldn't be created
(as was the case on my system), bubblewrap falls back to
/tmp/.bubblewrap-<UID>/. Local attacker could exploit this to prevent
other users from running bubblewrap, for example:

getent passwd | cut -d: -f3 | xargs printf '/tmp/.bubblewrap-%d\n' | xargs touch

But it gets worse, because bubblewrap is happy to use existing
/tmp/.bubblewrap-<UID>/, even when the directory is owned by some else.
In the worst case, this could be exploited by a local user to execute
arbitrary code in the container. (Though I couldn't find any way to
exploit this without disabling protected_symlinks.)

@smcv

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

This directory is a mount point for a tmpfs that only exists in bubblewrap's mount namespace, so I don't think its contents matter (which is why it's OK that all bubblewrap processes with the same uid use the
same directory): the first thing bubblewrap does with it is to mount a tmpfs over the top, which will be owned by the user running bubblewrap. If it's a symlink to a different directory (without protected_symlinks) then I can see some possible attack routes, which I won't go into on this public bug.

There are some tricky constraints on this directory. I think it might be created at a point in bubblewrap's lifetime where it shouldn't be trusting environment variables, so it can't fall back from $XDG_RUNTIME_DIR to $XDG_CACHE_HOME to $HOME/.cache like GLib does; but as far as I can see there's no good time for bubblewrap to delete the directory, so if it used mkstemp() you'd get one empty directory in /tmp for every bubblewrap run, which would never be deleted, which is eventually also denial of service.

Distribution packages can probably address this by creating an empty directory somewhere (in the package or in maintainer scripts) and making bubblewrap use that - because it mounts a tmpfs over the top, all it needs is a mount point somewhere out of the way, that you aren't ever going to want to expose in one of your bubblewrap-based containers (which rules out using /run, /tmp, /mnt, etc. as the mount point, because those are useful directories that you might want to share with the host).

@smcv

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

Distribution packages can probably address this by creating an empty directory somewhere (in the package or in maintainer scripts) and making bubblewrap use that

In fact it looks as though we can use any directory that is guaranteed to exist and be owned by root on any reasonable distribution, like /proc (which we already trust anyway), /tmp or /run, because by the time the container starts manipulating mount points we've already done the pivot_root dance.

smcv added a commit to smcv/bubblewrap that referenced this issue Mar 2, 2019

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. We already rely on /proc to have
those properties, so it seems as good a place as any. This doesn't appear
to have any impact on our ability to use /proc as either the source or
the destination of a bind-mount.

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

This comment has been minimized.

Copy link
Contributor Author

commented Mar 2, 2019

I would very much appreciate review on this. I'm preparing an upload to Debian unstable, but I don't think I'm comfortable with preparing stable updates with a backport of the solution I proposed until someone with a better overview of how bubblewrap uses pivot_root has reviewed it.

smcv added a commit to smcv/bubblewrap that referenced this issue Mar 5, 2019

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

rh-atomic-bot added a commit that referenced this issue Mar 5, 2019

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

rh-atomic-bot added a commit that referenced this issue Mar 5, 2019

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

rh-atomic-bot added a commit that referenced this issue Mar 6, 2019

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
@kdelee

This comment has been minimized.

Copy link

commented Apr 30, 2019

@smcv any plans to cut a release including this fix?

@smcv

This comment has been minimized.

Copy link
Contributor Author

commented May 1, 2019

I don't do releases for this project. @cgwalters?

@ret2libc

This comment has been minimized.

Copy link

commented May 27, 2019

I think this deserves a CVE. Has anyone already requested one? Any objections against having one allocated?

@cgwalters

This comment has been minimized.

Copy link
Member

commented May 27, 2019

For reference this was fixed with https://github.com/projectatomic/bubblewrap/releases/tag/v0.3.3

I think this deserves a CVE. Has anyone already requested one? Any objections against having one allocated?

No objections though I'd also note that I believe this to not be exploitable in most distributions (including RHEL) since XDG_RUNTIME_DIR will be set and we won't fall back to /tmp.

@ret2libc

This comment has been minimized.

Copy link

commented May 27, 2019

No objections though I'd also note that I believe this to not be exploitable in most distributions (including RHEL) since XDG_RUNTIME_DIR will be set and we won't fall back to /tmp.

I agree. I will request anyway one so that each distro can judge for itself.

@ret2libc

This comment has been minimized.

Copy link

commented May 29, 2019

CVE-2019-12439 was assigned to this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.