emo/procps
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

skill: Properly null-terminate buf in check_proc().

Browse Source 
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.
This commit is contained in:
Qualys Security Advisory 1970-01-01 00:00:00 +00:00 committed by Craig Small
parent 37547e9f5f
commit 52673d2fc7
1 changed files with 5 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
skill.c
View File

@ -176,6 +176,7 @@ static void check_proc(int pid, struct run_time_conf_t *run_time)
int tty; int tty;
int fd; int fd;
int i; int i;
ssize_t len;
if (pid == my_pid || pid == 0) if (pid == my_pid || pid == 0)
return; return;
/* pid (cmd) state ppid pgrp session tty */ /* 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) if (i == -1)
goto closure; goto closure;
} }
if (read(fd, buf, 128) <= 0) len = read(fd, buf, sizeof(buf));
goto closure; if (len <= 0 || (size_t)len >= sizeof(buf))
buf[127] = '\0'; goto closure;
buf[len] = '\0';
tmp = strrchr(buf, ')'); tmp = strrchr(buf, ')');
*tmp++ = '\0'; *tmp++ = '\0';
i = 5; i = 5;