]> granicus.if.org Git - procps-ng/commitdiff
0027-skill: Prevent multiple overflows in ENLIST().
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Sat, 23 Jun 2018 11:59:14 +0000 (21:59 +1000)
First problem: saved_argc was used to calculate the size of the array,
but saved_argc was never initialized. This triggers an immediate heap-
based buffer overflow:

$ skill -c0 -c0 -c0 -c0
Segmentation fault (core dumped)

Second problem: saved_argc was not the upper bound anyway, because one
argument can ENLIST() several times (for example, in parse_namespaces())
and overflow the array as well.

Third problem: integer overflow of the size of the array.

skill.c

diff --git a/skill.c b/skill.c
index 0f421d3b48197388788a9f21c3fefa893efced3d..3a92f09162bc6b818df32eb8be8acebb16c16893 100644 (file)
--- a/skill.c
+++ b/skill.c
@@ -65,7 +65,9 @@ static struct procps_namespaces match_namespaces;
 static int ns_flags = 0x3f;
 
 #define ENLIST(thing,addme) do{ \
-if(!thing##s) thing##s = xmalloc(sizeof(*thing##s)*saved_argc); \
+if(thing##_count < 0 || (size_t)thing##_count >= INT_MAX / sizeof(*thing##s)) \
+       xerrx(EXIT_FAILURE, _("integer overflow")); \
+thing##s = xrealloc(thing##s, sizeof(*thing##s)*(thing##_count+1)); \
 thing##s[thing##_count++] = addme; \
 }while(0)
 
@@ -82,7 +84,6 @@ enum rel_items {
     EU_PID, EU_EUID, EU_EUSER, EU_TTY, EU_TTYNAME, EU_CMD};
 
 static int my_pid;
-static int saved_argc;
 
 static int sig_or_pri;