]> granicus.if.org Git - procps-ng/commitdiff
proc/readproc.c: Harden read_unvectored().
authorQualys Security Advisory <qsa@qualys.com>
Thu, 1 Jan 1970 00:00:00 +0000 (00:00 +0000)
committerCraig Small <csmall@enc.com.au>
Fri, 18 May 2018 21:32:22 +0000 (07:32 +1000)
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

index 980ac4b06c04c10c1608f07e98033514c96b05d3..f63143d2a99e761a4251dd6874c807a94f9baaf8 100644 (file)
@@ -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;