From c7776e780425abb830c6b37c25cbd58ecc685680 Mon Sep 17 00:00:00 2001 From: Drew DeVault Date: Sat, 6 Oct 2018 12:17:36 -0400 Subject: [PATCH] Fix swaylock w/shadow on glibc, improve security Today I learned that GNU flaunts the POSIX standard in yet another creative way. Additionally, this adds some security improvements, namely: - Zeroing out password buffers in the privileged child process - setuid/setgid after reading /etc/shadow --- meson.build | 3 +++ shadow.c | 27 +++++++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/meson.build b/meson.build index 6605340..f3321a7 100644 --- a/meson.build +++ b/meson.build @@ -26,6 +26,9 @@ else warning('The swaylock binary must be setuid when compiled without libpam') warning('You must do this manually post-install: chmod a+s /path/to/swaylock') sources += ['shadow.c'] + if crypt.found() + dependencies += [crypt] + endif endif executable('swaylock', diff --git a/shadow.c b/shadow.c index 1f10514..f928eaa 100644 --- a/shadow.c +++ b/shadow.c @@ -6,9 +6,21 @@ #include #include #include "swaylock/swaylock.h" +#ifdef __GLIBC__ +// GNU, you damn slimy bastard +#include +#endif static int comm[2][2]; +static void clear_buffer(void *buf, size_t bytes) { + volatile char *buffer = buf; + volatile char zero = '\0'; + for (size_t i = 0; i < bytes; ++i) { + buffer[i] = zero; + } +} + void run_child(void) { /* This code runs as root */ struct passwd *pwent = getpwuid(getuid()); @@ -25,6 +37,17 @@ void run_child(void) { } encpw = swent->sp_pwdp; } + + /* We don't need any additional logging here because the parent process will + * also fail here and will handle logging for us. */ + if (setgid(getgid()) != 0) { + exit(EXIT_FAILURE); + } + if (setuid(getuid()) != 0) { + exit(EXIT_FAILURE); + } + + /* This code does not run as root */ wlr_log(WLR_DEBUG, "prepared to authorize user %s", pwent->pw_name); size_t size; @@ -60,10 +83,14 @@ void run_child(void) { result = strcmp(c, encpw) == 0; if (write(comm[1][1], &result, sizeof(result)) != sizeof(result)) { wlr_log_errno(WLR_ERROR, "failed to write pw check result"); + clear_buffer(buf, size); exit(EXIT_FAILURE); } + clear_buffer(buf, size); free(buf); } + + clear_buffer(encpw, strlen(encpw)); exit(EXIT_SUCCESS); }