From 50ae55ef1019a7851e87af4cb634aa054cc40340 Mon Sep 17 00:00:00 2001 From: Jim Warner Date: Sun, 3 Jun 2018 00:00:00 -0500 Subject: [PATCH] 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 --- proc/readproc.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/proc/readproc.c b/proc/readproc.c index 1b09d96c..0fc34cbe 100644 --- 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; }