]> granicus.if.org Git - procps-ng/commitdiff
pgrep: Prevent a potential stack-based buffer overflow.
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)
This is one of the worst issues that we found: if the strlen() of one of
the cmdline arguments is greater than INT_MAX (it is possible), then the
"int bytes" could wrap around completely, back to a very large positive
int, and the next strncat() would be called with a huge number of
destination bytes (a stack-based buffer overflow).

Fortunately, every distribution that we checked compiles its procps
utilities with FORTIFY, and the fortified strncat() detects and aborts
the buffer overflow before it occurs.

This patch also fixes a secondary issue: the old "--bytes;" meant that
cmdline[sizeof (cmdline) - 2] was never written to if the while loop was
never entered; in the example below, "ff" is the uninitialized byte:

((exec -ca `python3 -c 'print("A" * 131000)'` /usr/bin/cat < /dev/zero) | sleep 60) &
pgrep -a -P "$!" 2>/dev/null | hexdump -C
00000000  31 32 34 36 30 20 41 41  41 41 41 41 41 41 41 41  |12460 AAAAAAAAAA|
00000010  41 41 41 41 41 41 41 41  41 41 41 41 41 41 41 41  |AAAAAAAAAAAAAAAA|
*
00001000  41 41 41 41 ff 0a 31 32  34 36 32 20 73 6c 65 65  |AAAA..12462 slee|
00001010  70 20 36 30 0a                                    |p 60.|

pgrep.c

diff --git a/pgrep.c b/pgrep.c
index 91ab14147883c1ba74e10cf8e9b4e9be7eb752eb..65a7a7d28a330ec16bf1e62b6fd99838b42c0d0d 100644 (file)
--- a/pgrep.c
+++ b/pgrep.c
@@ -555,19 +555,24 @@ static struct el * select_procs (int *num)
                }
                if (task.cmdline && (opt_longlong || opt_full) ) {
                        int i = 0;
-                       int bytes = sizeof (cmdline) - 1;
+                       int bytes = sizeof (cmdline);
+                       char *str = cmdline;
 
                        /* make sure it is always NUL-terminated */
-                       cmdline[bytes] = 0;
-                       /* make room for SPC in loop below */
-                       --bytes;
-
-                       strncpy (cmdline, task.cmdline[i], bytes);
-                       bytes -= strlen (task.cmdline[i++]);
-                       while (task.cmdline[i] && bytes > 0) {
-                               strncat (cmdline, " ", bytes);
-                               strncat (cmdline, task.cmdline[i], bytes);
-                               bytes -= strlen (task.cmdline[i++]) + 1;
+                       *str = '\0';
+
+                       while (task.cmdline[i] && bytes > 1) {
+                               const int len = snprintf(str, bytes, "%s%s", i ? " " : "", task.cmdline[i]);
+                               if (len < 0) {
+                                       *str = '\0';
+                                       break;
+                               }
+                               if (len >= bytes) {
+                                       break;
+                               }
+                               str += len;
+                               bytes -= len;
+                               i++;
                        }
                }