public/nixpkgs
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Revert "Merge branch 'setuid-wrapper-readlink'"

Browse Source 
Kernel symlinks don't have st_size. Really thought I tested this, guess I ran the
wrong NixOS test :(

This reverts commit 6dab907ebe9c8015b8cbc4871313ed48c64c548c, reversing
changes made to eab479a5f0e46ad461ebda9953477be8f1e5e2bb.
This commit is contained in:
Shea Levy 2018-03-07 17:09:05 -05:00
parent 5a95fe2939
commit a183563cf0
No known key found for this signature in database
GPG Key ID: 5C0BD6957D86FE27
1 changed files with 11 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

36
nixos/modules/security/wrappers/wrapper.c
View File

@ -1,4 +1,3 @@
#define _GNU_SOURCE
#include <stdlib.h> #include <stdlib.h>
#include <stdio.h> #include <stdio.h>
#include <string.h> #include <string.h>
@ -162,34 +161,22 @@ static int make_caps_ambient(const char *selfPath)
int main(int argc, char * * argv) int main(int argc, char * * argv)
{ {
// O_PATH | O_NOFOLLOW gives us a fd pointing to the symlink // I *think* it's safe to assume that a path from a symbolic link
int selfExe = open("/proc/self/exe", O_PATH | O_CLOEXEC | O_NOFOLLOW); // should safely fit within the PATH_MAX system limit. Though I'm
assert(selfExe != -1); // not positive it's safe...
struct stat st; char selfPath[PATH_MAX];
assert(fstat(selfExe, &st) != -1); int selfPathSize = readlink("/proc/self/exe", selfPath, sizeof(selfPath));
size_t selfPathCap = st.st_size + 1;
char *selfPath = malloc(selfPathCap);
assert(selfPath);
int selfPathSize = readlinkat(selfExe, "", selfPath, selfPathCap);
assert(selfPathSize > 0); assert(selfPathSize > 0);
// Assert we have room for the zero byte, this ensures the path // Assert we have room for the zero byte, this ensures the path
// isn't being truncated because it's too big for the buffer. // isn't being truncated because it's too big for the buffer.
// //
// selfPathSize is the number of bytes readlinkat put into the // A better way to handle this might be to use something like the
// buffer, which does *not* append a null byte. selfPathCap is the // whereami library (https://github.com/gpakosz/whereami) or a
// capacity of the buffer, which was set to the number of bytes in // loop that resizes the buffer and re-reads the link if the
// the link contents (again, without the null byte) plus one for // contents are being truncated.
// the null byte. assert(selfPathSize < sizeof(selfPath));
//
// I don't think it's possible for the link contents to change
// between opening a symlink fd and readlinkat on it, so this is
// probably not necessary. Doubly so since this is /proc/self/exe,
// not a normal symlink. But it's trivial to check.
assert(selfPathSize < selfPathCap);
assert(close(selfExe));
// Set the zero byte since readlink doesn't do that for us. // Set the zero byte since readlink doesn't do that for us.
selfPath[selfPathSize] = '\0'; selfPath[selfPathSize] = '\0';
@ -210,6 +197,7 @@ int main(int argc, char * * argv)
// `selfPath', and not, say, as some other setuid program. That // `selfPath', and not, say, as some other setuid program. That
// is, our effective uid/gid should match the uid/gid of // is, our effective uid/gid should match the uid/gid of
// `selfPath'. // `selfPath'.
struct stat st;
assert(lstat(selfPath, &st) != -1); assert(lstat(selfPath, &st) != -1);
assert(!(st.st_mode & S_ISUID) || (st.st_uid == geteuid())); assert(!(st.st_mode & S_ISUID) || (st.st_uid == geteuid()));
@ -240,8 +228,6 @@ int main(int argc, char * * argv)
// capabilities too! // capabilities too!
make_caps_ambient(selfPath); make_caps_ambient(selfPath);
free(selfPath);
execve(sourceProg, argv, environ); execve(sourceProg, argv, environ);
fprintf(stderr, "%s: cannot run `%s': %s\n", fprintf(stderr, "%s: cannot run `%s': %s\n",