From 39dcf47bc85ad74dde0efa183f423820e2a90153 Mon Sep 17 00:00:00 2001 From: Qualys Security Advisory Date: Thu, 1 Jan 1970 00:00:00 +0000 Subject: [PATCH] proc/readproc.c: Harden read_unvectored(). 1/ Prevent an out-of-bounds write if sz is 0. 2/ Limit sz to INT_MAX, because the return value is an int, not an unsigned int (and because if INT_MAX is equal to SSIZE_MAX, man 2 read says "If count is greater than SSIZE_MAX, the result is unspecified.") 3/ Always null-terminate dst (unless sz is 0), because a return value of 0 because of an open() error (for example) is indistinguishable from a return value of 0 because of an empty file. 4/ Use an unsigned int for i (just like n), not an int. 5/ Check for snprintf() truncation. --- proc/readproc.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/proc/readproc.c b/proc/readproc.c index 980ac4b0..f63143d2 100644 --- a/proc/readproc.c +++ b/proc/readproc.c @@ -761,10 +761,15 @@ static char** file2strvec(const char* directory, const char* what) { // PROC_EDITCGRPCVT, PROC_EDITCMDLCVT and PROC_EDITENVRCVT static int read_unvectored(char *restrict const dst, unsigned sz, const char* whom, const char *what, char sep) { char path[PROCPATHLEN]; - int fd; + int fd, len; unsigned n = 0; - snprintf(path, sizeof(path), "%s/%s", whom, what); + if(sz <= 0) return 0; + if(sz >= INT_MAX) sz = INT_MAX-1; + dst[0] = '\0'; + + len = snprintf(path, sizeof(path), "%s/%s", whom, what); + if(len <= 0 || (size_t)len >= sizeof(path)) return 0; fd = open(path, O_RDONLY); if(fd==-1) return 0; @@ -774,16 +779,16 @@ static int read_unvectored(char *restrict const dst, unsigned sz, const char* wh if(errno==EINTR) continue; break; } + if(r<=0) break; // EOF n += r; if(n==sz) { // filled the buffer --n; // make room for '\0' break; } - if(r==0) break; // EOF } close(fd); if(n){ - int i=n; + unsigned i = n; while(i && dst[i-1]=='\0') --i; // skip trailing zeroes while(i--) if(dst[i]=='\n' || dst[i]=='\0') dst[i]=sep;