From: schwarze Date: Fri, 8 May 2015 12:30:27 +0000 (+0000) Subject: Fix about ten integer overflows and underflows and a handful of logic X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=e8da3a36c7f334c72bbd026a681be0ada55ee98a;p=openbsd Fix about ten integer overflows and underflows and a handful of logic errors in line number handling. Detailed explanations were sent to tech@ on October 18, 2014. OK doug@, and bapt at FreeBSD says he likes the direction --- diff --git a/usr.bin/col/col.c b/usr.bin/col/col.c index 42f4c61f97c..efceb690cba 100644 --- a/usr.bin/col/col.c +++ b/usr.bin/col/col.c @@ -1,4 +1,4 @@ -/* $OpenBSD: col.c,v 1.14 2014/10/17 21:27:10 schwarze Exp $ */ +/* $OpenBSD: col.c,v 1.15 2015/05/08 12:30:27 schwarze Exp $ */ /* $NetBSD: col.c,v 1.7 1995/09/02 05:48:50 jtc Exp $ */ /*- @@ -78,6 +78,7 @@ struct line_str { int l_needs_sort; /* set if chars went in out of order */ }; +void addto_lineno(int *, int); LINE *alloc_line(void); void dowarn(int); void flush_line(LINE *); @@ -91,7 +92,7 @@ CSET last_set; /* char_set of last char printed */ LINE *lines; int compress_spaces; /* if doing space -> tab conversion */ int fine; /* if `fine' resolution (half lines) */ -int max_bufd_lines; /* max # lines to keep in memory */ +int max_bufd_lines; /* max # of half lines to keep in memory */ int nblank_lines; /* # blanks after last flushed line */ int no_backspaces; /* if not to output any backspaces */ @@ -115,7 +116,7 @@ main(int argc, char *argv[]) int adjust, opt, warned; const char *errstr; - max_bufd_lines = 128; + max_bufd_lines = 256; compress_spaces = 1; /* compress spaces into tabs */ while ((opt = getopt(argc, argv, "bfhl:x")) != -1) switch (opt) { @@ -129,7 +130,8 @@ main(int argc, char *argv[]) compress_spaces = 1; break; case 'l': /* buffered line count */ - max_bufd_lines = strtonum(optarg, 1, INT_MAX, &errstr); + max_bufd_lines = strtonum(optarg, 1, + (INT_MAX - BUFFER_MARGIN) / 2, &errstr) * 2; if (errstr != NULL) errx(1, "bad -l argument, %s: %s", errstr, optarg); @@ -145,9 +147,6 @@ main(int argc, char *argv[]) if (optind != argc) usage(); - /* this value is in half lines */ - max_bufd_lines *= 2; - adjust = extra_lines = warned = 0; cur_col = 0; cur_line = max_line = nflushd_lines = this_line = 0; @@ -168,19 +167,19 @@ main(int argc, char *argv[]) case ESC: /* just ignore EOF */ switch(getchar()) { case RLF: - cur_line -= 2; + addto_lineno(&cur_line, -2); break; case RHLF: - cur_line--; + addto_lineno(&cur_line, -1); break; case FHLF: - cur_line++; + addto_lineno(&cur_line, 1); if (cur_line > max_line) max_line = cur_line; } continue; case NL: - cur_line += 2; + addto_lineno(&cur_line, 2); if (cur_line > max_line) max_line = cur_line; cur_col = 0; @@ -199,65 +198,68 @@ main(int argc, char *argv[]) ++cur_col; continue; case VT: - cur_line -= 2; + addto_lineno(&cur_line, -2); continue; } continue; } /* Must stuff ch in a line - are we at the right one? */ - if (cur_line != this_line - adjust) { + if (cur_line + adjust != this_line) { LINE *lnew; - int nmove; - - adjust = 0; - nmove = cur_line - this_line; - if (!fine) { - /* round up to next line */ - if (cur_line & 1) { - adjust = 1; - nmove++; - } - } - if (nmove < 0) { - for (; nmove < 0 && l->l_prev; nmove++) + + /* round up to next line */ + adjust = !fine && (cur_line & 1); + + if (cur_line + adjust < this_line) { + while (cur_line + adjust < this_line && + l->l_prev != NULL) { l = l->l_prev; - if (nmove) { + this_line--; + } + if (cur_line + adjust < this_line) { if (nflushd_lines == 0) { /* * Allow backup past first * line if nothing has been * flushed yet. */ - for (; nmove < 0; nmove++) { + while (cur_line + adjust + < this_line) { lnew = alloc_line(); l->l_prev = lnew; lnew->l_next = l; l = lines = lnew; extra_lines++; + this_line--; } } else { if (!warned++) dowarn(cur_line); - cur_line -= nmove; + cur_line = this_line - adjust; } } } else { /* may need to allocate here */ - for (; nmove > 0 && l->l_next; nmove--) + while (cur_line + adjust > this_line) { + if (l->l_next == NULL) { + l->l_next = alloc_line(); + l->l_next->l_prev = l; + } l = l->l_next; - for (; nmove > 0; nmove--) { - lnew = alloc_line(); - lnew->l_prev = l; - l->l_next = lnew; - l = lnew; + this_line++; } } - this_line = cur_line + adjust; - nmove = this_line - nflushd_lines; - if (nmove >= max_bufd_lines + BUFFER_MARGIN) { - nflushd_lines += nmove - max_bufd_lines; - flush_lines(nmove - max_bufd_lines); + if (this_line > nflushd_lines && + this_line - nflushd_lines >= + max_bufd_lines + BUFFER_MARGIN) { + if (extra_lines) { + flush_lines(extra_lines); + extra_lines = 0; + } + flush_lines(this_line - nflushd_lines - + max_bufd_lines); + nflushd_lines = this_line - max_bufd_lines; } } /* grow line's buffer? */ @@ -283,25 +285,23 @@ main(int argc, char *argv[]) l->l_max_col = cur_col; cur_col++; } - if (max_line == 0) - exit(0); /* no lines, so just exit */ + if (extra_lines) + flush_lines(extra_lines); /* goto the last line that had a character on it */ for (; l->l_next; l = l->l_next) this_line++; - flush_lines(this_line - nflushd_lines + extra_lines + 1); + flush_lines(this_line - nflushd_lines + 1); /* make sure we leave things in a sane state */ if (last_set != CS_NORMAL) PUTC('\017'); /* flush out the last few blank lines */ - nblank_lines = max_line - this_line; + if (max_line > this_line) + nblank_lines = max_line - this_line; if (max_line & 1) nblank_lines++; - else if (!nblank_lines) - /* missing a \n on the last line? */ - nblank_lines = 2; flush_blanks(); exit(0); } @@ -318,7 +318,8 @@ flush_lines(int nflush) flush_blanks(); flush_line(l); } - nblank_lines++; + if (l->l_line || l->l_next) + nblank_lines++; if (l->l_line) (void)free((void *)l->l_line); free_line(l); @@ -457,6 +458,23 @@ flush_line(LINE *l) } } +/* + * Increment or decrement a line number, checking for overflow. + * Stop one below INT_MAX such that the adjust variable is safe. + */ +void +addto_lineno(int *lno, int offset) +{ + if (offset > 0) { + if (*lno >= INT_MAX - offset) + errx(1, "too many lines"); + } else { + if (*lno < INT_MIN - offset) + errx(1, "too many reverse line feeds"); + } + *lno += offset; +} + #define NALLOC 64 static LINE *line_freelist;