From 1009d6e79e7f4ef92d7db27214c55a36f5e22c6f Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Thu, 27 Jan 2022 08:12:00 +0100 Subject: [PATCH 1/2] nixos/wrappers: create a new assert macro that always asserts C's assert macro only works when NDEBUG is undefined. Previously NDEBUG was undefined incorrectly which meant that the assert macros in wrapper.c did not work. --- nixos/modules/security/wrappers/wrapper.c | 37 ++++++++++++----------- 1 file changed, 20 insertions(+), 17 deletions(-) diff --git a/nixos/modules/security/wrappers/wrapper.c b/nixos/modules/security/wrappers/wrapper.c index 529669facda..49fc6c0ad53 100644 --- a/nixos/modules/security/wrappers/wrapper.c +++ b/nixos/modules/security/wrappers/wrapper.c @@ -2,12 +2,12 @@ #include #include #include +#include #include #include #include #include #include -#include #include #include #include @@ -16,10 +16,7 @@ #include #include -// Make sure assertions are not compiled out, we use them to codify -// invariants about this program and we want it to fail fast and -// loudly if they are violated. -#undef NDEBUG +#define ASSERT(expr) ((expr) ? (void) 0 : assert_failure(#expr)) extern char **environ; @@ -38,6 +35,12 @@ static char *wrapper_debug = "WRAPPER_DEBUG"; #define LE32_TO_H(x) (x) #endif +static noreturn void assert_failure(const char *assertion) { + fprintf(stderr, "Assertion `%s` in NixOS's wrapper.c failed.\n", assertion); + fflush(stderr); + abort(); +} + int get_last_cap(unsigned *last_cap) { FILE* file = fopen("/proc/sys/kernel/cap_last_cap", "r"); if (file == NULL) { @@ -181,36 +184,36 @@ int main(int argc, char **argv) { int len = strlen(wrapper_dir); if (len > 0 && '/' == wrapper_dir[len - 1]) --len; - assert(!strncmp(self_path, wrapper_dir, len)); - assert('/' == wrapper_dir[0]); - assert('/' == self_path[len]); + ASSERT(!strncmp(self_path, wrapper_dir, len)); + ASSERT('/' == wrapper_dir[0]); + ASSERT('/' == self_path[len]); // Make *really* *really* sure that we were executed as // `self_path', and not, say, as some other setuid program. That // is, our effective uid/gid should match the uid/gid of // `self_path'. struct stat st; - assert(lstat(self_path, &st) != -1); + ASSERT(lstat(self_path, &st) != -1); - assert(!(st.st_mode & S_ISUID) || (st.st_uid == geteuid())); - assert(!(st.st_mode & S_ISGID) || (st.st_gid == getegid())); + ASSERT(!(st.st_mode & S_ISUID) || (st.st_uid == geteuid())); + ASSERT(!(st.st_mode & S_ISGID) || (st.st_gid == getegid())); // And, of course, we shouldn't be writable. - assert(!(st.st_mode & (S_IWGRP | S_IWOTH))); + ASSERT(!(st.st_mode & (S_IWGRP | S_IWOTH))); // Read the path of the real (wrapped) program from .real. char real_fn[PATH_MAX + 10]; int real_fn_size = snprintf(real_fn, sizeof(real_fn), "%s.real", self_path); - assert(real_fn_size < sizeof(real_fn)); + ASSERT(real_fn_size < sizeof(real_fn)); int fd_self = open(real_fn, O_RDONLY); - assert(fd_self != -1); + ASSERT(fd_self != -1); char source_prog[PATH_MAX]; len = read(fd_self, source_prog, PATH_MAX); - assert(len != -1); - assert(len < sizeof(source_prog)); - assert(len > 0); + ASSERT(len != -1); + ASSERT(len < sizeof(source_prog)); + ASSERT(len > 0); source_prog[len] = 0; close(fd_self); From 2a6a3d2c47626782f604a1fb4ec506c834efb47a Mon Sep 17 00:00:00 2001 From: Konrad Borowski Date: Thu, 27 Jan 2022 08:14:53 +0100 Subject: [PATCH 2/2] nixos/wrappers: require argc to be at least one setuid applications were exploited in the past with an empty argv, such as pkexec using CVE-2021-4034. --- nixos/modules/security/wrappers/wrapper.c | 1 + 1 file changed, 1 insertion(+) diff --git a/nixos/modules/security/wrappers/wrapper.c b/nixos/modules/security/wrappers/wrapper.c index 49fc6c0ad53..a21ec500208 100644 --- a/nixos/modules/security/wrappers/wrapper.c +++ b/nixos/modules/security/wrappers/wrapper.c @@ -170,6 +170,7 @@ int readlink_malloc(const char *p, char **ret) { } int main(int argc, char **argv) { + ASSERT(argc >= 1); char *self_path = NULL; int self_path_size = readlink_malloc("/proc/self/exe", &self_path); if (self_path_size < 0) {