tech-userlevel archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[PATCH] grep: fix ASan heap-buffer-overflow.



Like the regular fgetln(), grep_fgetln() doesn't NUL-terminate the
string, which regexec() doesn't like. ASan just gained the ability to
intercept regexec(), which is why we didn't find this previously.

Bug: http://b/129089665
Test: adb shell grep -R /system -e "abc"
---
 toolbox/upstream-netbsd/usr.bin/grep/file.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/toolbox/upstream-netbsd/usr.bin/grep/file.c
b/toolbox/upstream-netbsd/usr.bin/grep/file.c
index ef057ba15..73043a24b 100644
--- a/toolbox/upstream-netbsd/usr.bin/grep/file.c
+++ b/toolbox/upstream-netbsd/usr.bin/grep/file.c
@@ -63,7 +63,7 @@ static gzFile gzbufdesc;
 static BZFILE* bzbufdesc;
 #endif

-static unsigned char buffer[MAXBUFSIZ];
+static unsigned char buffer[MAXBUFSIZ + 1];
 static unsigned char *bufpos;
 static size_t bufrem;

@@ -166,6 +166,7 @@ grep_fgetln(struct file *f, size_t *lenp)
  len = p - bufpos;
  bufrem -= len;
  bufpos = p;
+ *bufpos = '\0';
  *lenp = len;
  return (ret);
  }
@@ -176,6 +177,7 @@ grep_fgetln(struct file *f, size_t *lenp)
  if (grep_lnbufgrow(len + LNBUFBUMP))
  goto error;
  memcpy(lnbuf + off, bufpos, len - off);
+ lnbuf[off + len - off] = '\0';
  off = len;
  if (grep_refill(f) != 0)
  goto error;
@@ -188,9 +190,10 @@ grep_fgetln(struct file *f, size_t *lenp)
  ++p;
  diff = p - bufpos;
  len += diff;
- if (grep_lnbufgrow(len))
+ if (grep_lnbufgrow(len + 1))
      goto error;
  memcpy(lnbuf + off, bufpos, diff);
+ lnbuf[off+diff] = '\0';
  bufrem -= diff;
  bufpos = p;
  break;
-- 
2.21.0.392.gf8f6787159e-goog


obviously that's a patch against Android's copy of NetBSD grep. that's
the one i've actually tested.

here's one against NetBSD itself:

Index: file.c
===================================================================
RCS file: /cvsroot/src/usr.bin/grep/file.c,v
retrieving revision 1.10
diff -u -r1.10 file.c
--- file.c 12 Aug 2018 09:03:21 -0000 1.10
+++ file.c 30 Mar 2019 05:08:17 -0000
@@ -63,7 +63,7 @@
 static BZFILE* bzbufdesc;
 #endif

-static unsigned char buffer[MAXBUFSIZ];
+static unsigned char buffer[MAXBUFSIZ + 1];
 static unsigned char *bufpos;
 static size_t bufrem;

@@ -166,6 +166,7 @@
  len = p - bufpos;
  bufrem -= len;
  bufpos = p;
+ *bufpos = '\0';
  *lenp = len;
  return (ret);
  }
@@ -176,6 +177,7 @@
  if (grep_lnbufgrow(len + LNBUFBUMP))
  goto error;
  memcpy(lnbuf + off, bufpos, len - off);
+ lnbuf[off + len - off] = '\0';
  off = len;
  if (grep_refill(f) != 0)
  goto error;
@@ -188,9 +190,10 @@
  ++p;
  diff = p - bufpos;
  len += diff;
- if (grep_lnbufgrow(len))
+ if (grep_lnbufgrow(len + 1))
      goto error;
  memcpy(lnbuf + off, bufpos, diff);
+ lnbuf[off+diff] = '\0';
  bufrem -= diff;
  bufpos = p;
  break;

but i haven't actually built NetBSD to test it in situ. fixes the
crashes on asan-ified Android though. BSD patch attached, since i have
no doubt that gmail will remove all the tabs despite being in "plain
text" mode.
Index: file.c
===================================================================
RCS file: /cvsroot/src/usr.bin/grep/file.c,v
retrieving revision 1.10
diff -u -r1.10 file.c
--- file.c	12 Aug 2018 09:03:21 -0000	1.10
+++ file.c	30 Mar 2019 05:08:27 -0000
@@ -63,7 +63,7 @@
 static BZFILE* bzbufdesc;
 #endif
 
-static unsigned char buffer[MAXBUFSIZ];
+static unsigned char buffer[MAXBUFSIZ + 1];
 static unsigned char *bufpos;
 static size_t bufrem;
 
@@ -166,6 +166,7 @@
 		len = p - bufpos;
 		bufrem -= len;
 		bufpos = p;
+		*bufpos = '\0';
 		*lenp = len;
 		return (ret);
 	}
@@ -176,6 +177,7 @@
 		if (grep_lnbufgrow(len + LNBUFBUMP))
 			goto error;
 		memcpy(lnbuf + off, bufpos, len - off);
+		lnbuf[off + len - off] = '\0';
 		off = len;
 		if (grep_refill(f) != 0)
 			goto error;
@@ -188,9 +190,10 @@
 		++p;
 		diff = p - bufpos;
 		len += diff;
-		if (grep_lnbufgrow(len))
+		if (grep_lnbufgrow(len + 1))
 		    goto error;
 		memcpy(lnbuf + off, bufpos, diff);
+		lnbuf[off+diff] = '\0';
 		bufrem -= diff;
 		bufpos = p;
 		break;


Home | Main Index | Thread Index | Old Index