diff options
author | philhofer <phofer@umich.edu> | 2018-12-18 20:36:26 -0800 |
---|---|---|
committer | William Hubbs <w.d.hubbs@gmail.com> | 2018-12-24 11:55:48 -0600 |
commit | 084877eb52971faf8f52c780ddd08ed9af140eb6 (patch) | |
tree | b93d79d93039fbe27f9a58fd011e4a8ff27454d0 | |
parent | 97e74f97347f5798e01a47057efab00906754546 (diff) |
src/librc/librc-daemon.c: fix buffer overrun in pid_is_argv
The contents of /proc/<pid>/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/<pid>/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.
-rw-r--r-- | src/librc/librc-daemon.c | 34 |
1 files 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 * |