]> granicus.if.org Git - procps-ng/commitdiff
skill: Properly null-terminate buf in check_proc().
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Fri, 18 May 2018 21:32:21 +0000 (07:32 +1000)
Right now, if read() returns less than 127 bytes (the most likely case),
the end of the "string" buf will contain garbage from the stack, because
buf is always null-terminated at a fixed offset 127. This is especially
bad because the next operation is a strrchr().

Also, make sure that the whole /proc/PID/stat file is read, otherwise
its parsing may be unsafe (the strrchr() may point into user-controlled
data, comm). This should never happen with the current file format (comm
is very short), but be safe, just in case.

skill.c

diff --git a/skill.c b/skill.c
index f32e918e620e777b15dd214f1a762a0617d6bc92..27a1dd772d8b8897f4de33f7abe7b3ebad985979 100644 (file)
--- a/skill.c
+++ b/skill.c
@@ -176,6 +176,7 @@ static void check_proc(int pid, struct run_time_conf_t *run_time)
        int tty;
        int fd;
        int i;
+       ssize_t len;
        if (pid == my_pid || pid == 0)
                return;
        /* pid (cmd) state ppid pgrp session tty */
@@ -198,9 +199,10 @@ static void check_proc(int pid, struct run_time_conf_t *run_time)
                if (i == -1)
                        goto closure;
        }
-       if (read(fd, buf, 128) <= 0)
-           goto closure;
-       buf[127] = '\0';
+       len = read(fd, buf, sizeof(buf));
+       if (len <= 0 || (size_t)len >= sizeof(buf))
+               goto closure;
+       buf[len] = '\0';
        tmp = strrchr(buf, ')');
        *tmp++ = '\0';
        i = 5;