On Wed, Oct 17, 2018 at 10:49:26AM -0600, Theo de Raadt wrote:
> Should this use reallocarray() instead, to catch *2 overflow?
>
> (Really it will crash at that point. But using reallocarray can
> identify it with a clean error)
>
> > The bug is in fmt. If len == length the buf[len] = '\0' statement is
> > an overflow, which happens if the line is exactly 100 chars long.
Something like this?
-Otto
Index: fmt.c
===================================================================
RCS file: /cvs/src/usr.bin/fmt/fmt.c,v
retrieving revision 1.38
diff -u -p -r1.38 fmt.c
--- fmt.c 20 Feb 2017 15:48:00 -0000 1.38
+++ fmt.c 17 Oct 2018 17:53:15 -0000
@@ -245,7 +245,7 @@ static void output_word(size_t, size_t,
static void output_indent(size_t);
static void center_stream(FILE *, const char *);
static char *get_line(FILE *);
-static void *xrealloc(void *, size_t);
+static void *xreallocarray(void *, size_t, size_t);
void usage(void);
#define ERRS(x) (x >= 127 ? 127 : ++x)
@@ -680,16 +680,16 @@ get_line(FILE *stream)
if (buf == NULL) {
length = 100;
- buf = xrealloc(NULL, length);
+ buf = xreallocarray(NULL, length, 1);
}
while ((ch = getc(stream)) != '\n' && ch != EOF) {
if ((len == 0) && (ch == '.' && !format_troff))
troff = 1;
if (troff || ch == '\t' || !iscntrl(ch)) {
- if (len >= length) {
+ if (len >= length - 1) {
+ buf = xreallocarray(buf, length, 2);
length *= 2;
- buf = xrealloc(buf, length);
}
buf[len++] = ch;
} else if (ch == '\b') {
@@ -706,11 +706,11 @@ get_line(FILE *stream)
/* (Re)allocate some memory, exiting with an error if we can't.
*/
static void *
-xrealloc(void *ptr, size_t nbytes)
+xreallocarray(void *ptr, size_t nmemb, size_t size)
{
void *p;
- p = realloc(ptr, nbytes);
+ p = reallocarray(ptr, nmemb, size);
if (p == NULL)
errx(1, "out of memory");
return p;