From 807498f89926ea28b6ad874dafe1e902604a2426 Mon Sep 17 00:00:00 2001 From: Qualys Security Advisory Date: Thu, 1 Jan 1970 00:00:00 +0000 Subject: [PATCH] 0070-proc/readproc.c: Harden status2proc(). 1/ Do not read past the terminating null byte when hashing the name. 2/ S[x] is used as an index, but S is "char *S" (signed) and hence may index the array out-of-bounds. Bit-mask S[x] with 127 (the array has 128 entries). 3/ Use a size_t for j, not an int (strlen() returns a size_t). Notes: - These are (mostly) theoretical problems, because the contents of /proc/PID/status are (mostly) trusted. - The "name" member of the status_table_struct has 8 bytes, and "RssShmem" occupies exactly 8 bytes, which means that "name" is not null-terminated. This is fine right now, because status2proc() uses memcmp(), not strcmp(), but it is worth mentioning. ---------------------------- adapted for newlib branch . newlib doesn't use that 'unlikely' crap . newlib also had a '#ifdef FALSE_THREADS' Signed-off-by: Jim Warner --- proc/readproc.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/proc/readproc.c b/proc/readproc.c index 6144fc02..a53f0775 100644 --- a/proc/readproc.c +++ b/proc/readproc.c @@ -36,6 +36,7 @@ #include #include #include +#include #ifdef WITH_SYSTEMD #include #endif @@ -249,7 +250,8 @@ ENTER(0x220); // examine a field name (hash and compare) base: if(!*S) break; - entry = table[(GPERF_TABLE_SIZE -1) & (asso[(int)S[3]] + asso[(int)S[2]] + asso[(int)S[0]])]; + if((!S[0] || !S[1] || !S[2] || !S[3])) break; + entry = table[(GPERF_TABLE_SIZE -1) & (asso[S[3]&127] + asso[S[2]&127] + asso[S[0]&127])]; colon = strchr(S, ':'); if(!colon) break; if(colon[1]!='\t') break; @@ -385,12 +387,12 @@ ENTER(0x220); continue; case_Groups: { char *nl = strchr(S, '\n'); - int j = nl ? (nl - S) : strlen(S); + size_t j = nl ? (size_t)(nl - S) : strlen(S); #ifdef FALSE_THREADS - if (j && !IS_THREAD(P)) { + if (!IS_THREAD(P) && j > 0 && j < INT_MAX) { #else - if (j) { + if (j > 0 && j < INT_MAX) { #endif P->supgid = malloc(j+1); // +1 in case space disappears if (!P->supgid)