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.
This commit is contained in:
Qualys Security Advisory 1970-01-01 00:00:00 +00:00 committed by Craig Small
parent 36c350f07c
commit 39dcf47bc8

View File

@ -761,10 +761,15 @@ static char** file2strvec(const char* directory, const char* what) {
// PROC_EDITCGRPCVT, PROC_EDITCMDLCVT and PROC_EDITENVRCVT // 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) { static int read_unvectored(char *restrict const dst, unsigned sz, const char* whom, const char *what, char sep) {
char path[PROCPATHLEN]; char path[PROCPATHLEN];
int fd; int fd, len;
unsigned n = 0; 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); fd = open(path, O_RDONLY);
if(fd==-1) return 0; 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; if(errno==EINTR) continue;
break; break;
} }
if(r<=0) break; // EOF
n += r; n += r;
if(n==sz) { // filled the buffer if(n==sz) { // filled the buffer
--n; // make room for '\0' --n; // make room for '\0'
break; break;
} }
if(r==0) break; // EOF
} }
close(fd); close(fd);
if(n){ if(n){
int i=n; unsigned i = n;
while(i && dst[i-1]=='\0') --i; // skip trailing zeroes while(i && dst[i-1]=='\0') --i; // skip trailing zeroes
while(i--) while(i--)
if(dst[i]=='\n' || dst[i]=='\0') dst[i]=sep; if(dst[i]=='\n' || dst[i]=='\0') dst[i]=sep;