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

Use of newfstatat on newer glibc #3

Open
bemoody opened this issue Jun 20, 2023 · 2 comments
Open

Use of newfstatat on newer glibc #3

bemoody opened this issue Jun 20, 2023 · 2 comments

Comments

@bemoody
Copy link
Owner

bemoody commented Jun 20, 2023

sandboxed-lightwave (0.71) works on glibc 2.31-13+deb11u6 (bullseye) and doesn't work on glibc 2.36-9 (bookworm).

The reason is that glibc now uses the newfstatat system call, instead of fstat or fstat64, to implement the fstat library function.

(The LightWAVE sandbox can't allow newfstatat(fd, "", &s, AT_EMPTY_PATH) - equivalent to fstat(fd, &s), which is safe - without also allowing newfstatat(fd, "foo", &s, AT_EMPTY_PATH) - roughly equivalent to fchdir(fd), stat("foo", &s), which is dangerous. The design of AT_EMPTY_PATH is silly.)

Now, this is really only a theoretical problem, for a bunch of reasons:

  • As far as I know, neither WFDB nor LightWAVE currently requires fstat. libFLAC does, but not if the caller provides its own file I/O callbacks (which WFDB does.) glibc uses fstat in an advisory fashion and has sensible fallback behavior when it fails.

  • If sandboxed-lightwave is invoked by a trusted caller (which can guarantee not to pass any directory file descriptors), then there isn't any problem with allowing newfstatat. Note, for example, that physionet-build uses the close_fds argument to subprocess.Popen. The sandbox doesn't allow execve, and sets no-new-privs, so sandboxed-lightwave can't invoke itself.

  • The reason directory FDs are dangerous to begin with is that they allow escaping from an external chroot. We don't use chroot for security at the system level, and probably anybody else should be using a filesystem namespace rather than chroot alone.

  • On PhysioNet, if somebody is able to find some incredibly complicated exploit that lets them reveal the existence / size / modification time of some known filesystem path? Big deal.

That said, I don't like the idea of releasing lightwave with a known security vulnerability. These are the options I see:

  • Make newfstatat fail with ENOSYS or something.

  • Use lightwave in unprivileged (user-namespace) mode.

  • Link sandboxed-lightwave with some other libc, or with a hacked version of glibc.

  • Add more complicated contortions to sandbox.c, to prevent callers from passing any directory FD.

  • Add more complicated contortions to sandbox.c, to refuse to run in a chrooted environment.

@bemoody
Copy link
Owner Author

bemoody commented Jun 20, 2023

or:

  • Dark magic using SCMP_ACT_TRAP to emulate the system call.

@bemoody
Copy link
Owner Author

bemoody commented Oct 6, 2023

For what it's worth, this issue has supposedly been fixed in glibc master, and presumably will be in 2.39, on some, but not all architectures: https://sourceware.org/git/?p=glibc.git;a=commitdiff;h=551101e8240b7514fc646d1722f8b79c90362b8f

According to commenters on LWN (https://lwn.net/Articles/944214/), there isn't currently a way to fix this for all architectures: fstat isn't Y2038-safe on 32-bit architectures, and some newer architectures don't have fstat at all. As the LWN article notes, this is also a performance issue, so maybe a future kernel will rectify it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant