emo/procps
1
0
Fork 0
Code Issues Pull Requests Packages Releases Activity

proc/readproc.c: Harden file2str().

Browse Source 
1/ Replace sprintf() with snprintf() (and check for truncation).

2/ Prevent an integer overflow of ub->siz. The "tot_read--" is needed to
avoid an off-by-one overflow in "ub->buf[tot_read] = '\0'". It is safe
to decrement tot_read here, because we know that tot_read is equal to
ub->siz (and ub->siz is very large).

We believe that truncation is a better option than failure (implementing
failure instead should be as easy as replacing the "tot_read--" with
"tot_read = 0").
This commit is contained in:
Qualys Security Advisory 1970-01-01 00:00:00 +00:00 committed by Craig Small
parent 344f6d3c0e
commit ccf8de0874
1 changed files with 7 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

9
proc/readproc.c
View File

@ -658,7 +658,7 @@ static void statm2proc(const char* s, proc_t *restrict P) {
static int file2str(const char *directory, const char *what, struct utlbuf_s *ub) {
#define buffGRW 1024
char path[PROCPATHLEN];
int fd, num, tot_read = 0;
int fd, num, tot_read = 0, len;
/* on first use we preallocate a buffer of minimum size to emulate
former 'local static' behavior -- even if this read fails, that
@ -666,11 +666,16 @@ static int file2str(const char *directory, const char *what, struct utlbuf_s *ub
( besides, with this xcalloc we will never need to use memcpy ) */
if (ub->buf) ub->buf[0] = '\0';
else ub->buf = xcalloc((ub->siz = buffGRW));
sprintf(path, "%s/%s", directory, what);
len = snprintf(path, sizeof path, "%s/%s", directory, what);
if (len <= 0 || (size_t)len >= sizeof path) return -1;
if (-1 == (fd = open(path, O_RDONLY, 0))) return -1;
while (0 < (num = read(fd, ub->buf + tot_read, ub->siz - tot_read))) {
tot_read += num;
if (tot_read < ub->siz) break;
if (ub->siz >= INT_MAX - buffGRW) {
tot_read--;
break;
}
ub->buf = xrealloc(ub->buf, (ub->siz += buffGRW));
};
ub->buf[tot_read] = '\0';