]> granicus.if.org Git - procps-ng/commitdiff
0075-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>
Sat, 9 Jun 2018 11:35:19 +0000 (21:35 +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.

---------------------------- adapted for newlib branch
. adapted via 'patch (without rejections)

Signed-off-by: Jim Warner <james.warner@comcast.net>
proc/readproc.c

index 1a8019508204639ae03f690ba98975df2794faf5..72794d2b2c015bf2790df06c1c45fd0fa0a4ea2b 100644 (file)
@@ -763,10 +763,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;
 
@@ -776,16 +781,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;