From 81c3f64110bb76e24d6062eafd7206c10f676d6f Mon Sep 17 00:00:00 2001 From: Alex Richardson Date: Tue, 19 Jan 2021 11:35:07 +0000 Subject: [PATCH] usr.bin/grep: Fix Address OOB read error I found this when compiling all the bootstrap tools with -fsanitize=addres: ==65590==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x62d000008400 at pc 0x000000473053 bp 0x7ffc1c7dd910 sp 0x7ffc1c7dd0b8 READ of size 32769 at 0x62d000008400 thread T0 #0 0x473052 in regexec (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x473052) #1 0x4c9cf3 in procline /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:539:8 #2 0x4c8687 in procfile /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:379:18 #3 0x4c6596 in main /local/scratch/alr48/cheri/freebsd/usr.bin/grep/grep.c:714:8 0x62d000008400 is located 0 bytes to the right of 32768-byte region [0x62d000000400,0x62d000008400) allocated by thread T0 here: #0 0x493d5d in malloc (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x493d5d) #1 0x4cad75 in grep_malloc /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c:656:13 #2 0x4c8129 in procfile /local/scratch/alr48/cheri/freebsd/usr.bin/grep/util.c #3 0x4c6596 in main /local/scratch/alr48/cheri/freebsd/usr.bin/grep/grep.c:714:8 SUMMARY: AddressSanitizer: heap-buffer-overflow (/local/scratch/alr48/cheri/build/freebsd-amd64-build/local/scratch/alr48/cheri/freebsd/amd64.amd64/tmp/legacy/bin/grep+0x473052) in regexec Reviewed By: kevans MFC after: 1 week --- usr.bin/grep/file.c | 11 +++++++---- usr.bin/grep/queue.c | 4 +++- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/usr.bin/grep/file.c b/usr.bin/grep/file.c index 3d86bef010e5..8577572c2887 100644 --- a/usr.bin/grep/file.c +++ b/usr.bin/grep/file.c @@ -98,7 +98,6 @@ char * grep_fgetln(struct file *f, struct parsec *pc) { char *p; - char *ret; size_t len; size_t off; ptrdiff_t diff; @@ -116,12 +115,15 @@ grep_fgetln(struct file *f, struct parsec *pc) /* Look for a newline in the remaining part of the buffer */ if ((p = memchr(bufpos, fileeol, bufrem)) != NULL) { ++p; /* advance over newline */ - ret = bufpos; len = p - bufpos; + if (grep_lnbufgrow(len + 1)) + goto error; + memcpy(lnbuf, bufpos, len); bufrem -= len; bufpos = p; pc->ln.len = len; - return (ret); + lnbuf[len] = '\0'; + return (lnbuf); } /* We have to copy the current buffered data to the line buffer */ @@ -148,7 +150,7 @@ grep_fgetln(struct file *f, struct parsec *pc) ++p; diff = p - bufpos; len += diff; - if (grep_lnbufgrow(len)) + if (grep_lnbufgrow(len + 1)) goto error; memcpy(lnbuf + off, bufpos, diff); bufrem -= diff; @@ -156,6 +158,7 @@ grep_fgetln(struct file *f, struct parsec *pc) break; } pc->ln.len = len; + lnbuf[len] = '\0'; return (lnbuf); error: diff --git a/usr.bin/grep/queue.c b/usr.bin/grep/queue.c index ac15185f0694..9babdbf74af8 100644 --- a/usr.bin/grep/queue.c +++ b/usr.bin/grep/queue.c @@ -95,12 +95,14 @@ enqueue(struct str *x) rotated = true; free(item->dat); } - item->dat = grep_malloc(sizeof(char) * x->len); + /* len + 1 for NUL-terminator */ + item->dat = grep_malloc(sizeof(char) * x->len + 1); item->len = x->len; item->line_no = x->line_no; item->boff = x->boff; item->off = x->off; memcpy(item->dat, x->dat, x->len); + item->dat[x->len] = '\0'; item->file = x->file; return (rotated);