library: avoid problems involving 'supgid' mishandling

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>

diff --git a/proc/readproc.c b/proc/readproc.c
index 1b09d96ccfda96cf9a170dbceac5bb36a7f892fb..0fc34cbeaf4a48ff2e34e1f95e3001988f98e5fa 100644 (file)
--- a/proc/readproc.c
+++ b/proc/readproc.c
@@ -393,17 +393,20 @@ ENTER(0x220);
         P->vm_swap = (unsigned long)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;
 
 #ifdef FALSE_THREADS
         if (IS_THREAD(P)) continue;
 #endif
+        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 = malloc(j+1);        // +1 in case space disappears
             if (!P->supgid)
                 return 1;
-            memcpy(P->supgid, S, j);
+            memcpy(P->supgid, ss, j);
             if (' ' != P->supgid[--j]) ++j;
             P->supgid[j] = '\0';            // whack the space or the newline
             for ( ; j; j--)
@@ -482,11 +485,9 @@ static int supgrps_from_supgids (proc_t *p) {
 #ifdef FALSE_THREADS
     if (IS_THREAD(p)) return 0;
 #endif
-    if (!p->supgid || '-' == *p->supgid) {
-        if (!(p->supgrp = strdup("-")))
-            return 1;
-        return 0;
-    }
+    if (!p->supgid || '-' == *p->supgid)
+        goto wrap_up;
+
     s = p->supgid;
     t = 0;
     do {
@@ -511,6 +512,10 @@ static int supgrps_from_supgids (proc_t *p) {
         t += len;
     } while (*s);
 
+wrap_up:
+    if (!p->supgrp
+    && !(p->supgrp = strdup("-")))
+        return 1;
     return 0;
 }