]> granicus.if.org Git - procps-ng/commitdiff
library: avoid problems involving 'supgid' mishandling
authorJim Warner <james.warner@comcast.net>
Wed, 30 May 2018 05:00:00 +0000 (00:00 -0500)
committerCraig Small <csmall@enc.com.au>
Thu, 31 May 2018 12:02:03 +0000 (22:02 +1000)
Following that patch referenced below, the top SUPGRPS
field would produce a segmentation fault and ps SUPGRP
would often show "(null)". Such problems resulted from
some faulty logic in the status2proc() routine dealing
with 'Groups' (supgid) which served as a source field.

For many processes the original code produced an empty
string which prevented conversion to the expected "-".
Moreover, prior to release 3.3.15 such an empty string
will become 0 after strtol() which pwcache_get_group()
translates to 'root' yielding very misleading results.

So, now we'll check for empty '/proc/#/status/Groups:'
fields & consistently provide a "-" value for callers.

[ we'll also protect against future problems in that ]
[ new qualys logic by always ensuring valid 'supgrp' ]
[ pointers - logic which revealed our original flaw! ]

Reference(s):
. original qualys patch
0071-proc-readproc.c-Harden-supgrps_from_supgids.patch

Signed-off-by: Jim Warner <james.warner@comcast.net>
proc/readproc.c

index 0f002315ecaa6e150d32736ec45671548a66f79b..ea7a31bb95122a1751982c4154412f90ec21d95d 100644 (file)
@@ -387,12 +387,15 @@ ENTER(0x220);
         P->vm_swap = strtol(S,&S,10);
         continue;
     case_Groups:
-    {   char *nl = strchr(S, '\n');
-        size_t j = nl ? (size_t)(nl - S) : strlen(S);
+    {   char *ss = S, *nl = strchr(S, '\n');
+        size_t j;
 
+        while (' ' == *ss || '\t' == *ss) ss++;
+        if (ss >= nl) continue;
+        j = nl ? (size_t)(nl - ss) : strlen(ss);
         if (j > 0 && j < INT_MAX) {
             P->supgid = xmalloc(j+1);       // +1 in case space disappears
-            memcpy(P->supgid, S, j);
+            memcpy(P->supgid, ss, j);
             if (unlikely(' ' != P->supgid[--j])) ++j;
             P->supgid[j] = '\0';            // whack the space or the newline
             for ( ; j; j--)
@@ -472,7 +475,11 @@ static void supgrps_from_supgids (proc_t *p) {
 
         while (',' == *s) ++s;
         gid = strtol(s, &end, 10);
-        if (end <= s) break;
+        if (end <= s) {
+            if (!p->supgrp)
+                p->supgrp = xstrdup("-");
+            break;
+        }
         s = end;
         g = pwcache_get_group(gid);