From 084877eb52971faf8f52c780ddd08ed9af140eb6 Mon Sep 17 00:00:00 2001 From: philhofer Date: Tue, 18 Dec 2018 20:36:26 -0800 Subject: [PATCH] src/librc/librc-daemon.c: fix buffer overrun in pid_is_argv The contents of /proc//cmdline are read into a stack buffer using bytes = read(fd, buffer, sizeof(buffer)); followed by appending a null terminator to the buffer with buffer[bytes] = '\0'; If bytes == sizeof(buffer), then this write is out-of-bounds. Refactor the code to use rc_getfile instead, since PATH_MAX is not the maximum size of /proc//cmdline. (I hit this issue in practice while compiling Linux; it tripped the stack-smashing protector.) This is roughly the same buffer overflow condition that was fixed by commit 0ddee9b7d2b8dea810e252ca6a95c457876df120 This fixes #269. --- src/librc/librc-daemon.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/src/librc/librc-daemon.c b/src/librc/librc-daemon.c index 6f3b492f..2b0d9712 100644 --- a/src/librc/librc-daemon.c +++ b/src/librc/librc-daemon.c @@ -48,34 +48,40 @@ pid_is_exec(pid_t pid, const char *exec) static bool pid_is_argv(pid_t pid, const char *const *argv) { + char *buffer = NULL; char *cmdline = NULL; - int fd; - char buffer[PATH_MAX]; char *p; - ssize_t bytes; + size_t bytes; + bool rc; xasprintf(&cmdline, "/proc/%u/cmdline", pid); - if ((fd = open(cmdline, O_RDONLY)) < 0) { + if (!rc_getfile(cmdline, &buffer, &bytes)) { free(cmdline); return false; } - bytes = read(fd, buffer, sizeof(buffer)); - close(fd); free(cmdline); - if (bytes == -1) + if (bytes <= 0) { + if (buffer) + free(buffer); return false; - - buffer[bytes] = '\0'; + } p = buffer; + rc = true; while (*argv) { - if (strcmp(*argv, p) != 0) - return false; + if (strcmp(*argv, p) != 0) { + rc = false; + break; + } + argv++; p += strlen(p) + 1; - if ((unsigned)(p - buffer) > sizeof(buffer)) - return false; + if ((unsigned)(p - buffer) >= bytes) { + rc = false; + break; + } } - return true; + free(buffer); + return rc; } RC_PIDLIST *