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;

Reply via email to