From: Qualys Security Advisory <qsa@qualys.com>
Date: Thu, 1 Jan 1970 00:00:00 +0000 (+0000)
Subject: skill: Properly null-terminate buf in check_proc().
X-Git-Tag: v3.3.15~113
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=52673d2fc7e012f5134cdfafb6d319450b3a40a3;p=procps-ng

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

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.
---

diff --git a/skill.c b/skill.c
index f32e918e..27a1dd77 100644
--- 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;