On 10/2/19 7:50 AM, Roland Illig wrote:
The current code says:next_line_no = line_no + page_incr; if (next_line_no < line_no) die (EXIT_FAILURE, 0, _("line number overflow")); Since intmax_t is a regular integer type, overflow invokes undefined behavior and must therefore be checked using other means.
Thanks for the bug report. I looked for similar problems involving integer-overflow diagnostics in coreutils and installed the attached patches. The second patch should fix the bug you mentioned.
>From 1316620e81daf91317560226b2b63cbbf548c09d Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Thu, 3 Oct 2019 12:35:44 -0700 Subject: [PATCH 1/4] cp: simplify integer overflow checking * src/copy.c (sparse_copy): Use INT_ADD_WRAPV instead of doing overflow checking by hand. --- src/copy.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/copy.c b/src/copy.c index 65cf65895..cd6104c7a 100644 --- a/src/copy.c +++ b/src/copy.c @@ -335,9 +335,7 @@ sparse_copy (int src_fd, int dest_fd, char *buf, size_t buf_size, } else /* Coalesce writes/seeks. */ { - if (psize <= OFF_T_MAX - csize) - psize += csize; - else + if (INT_ADD_WRAPV (psize, csize, &psize)) { error (0, 0, _("overflow reading %s"), quoteaf (src_name)); return false; -- 2.21.0
>From 89af2b307b455b53869bc9cf79af0272f7d8a1a2 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Thu, 3 Oct 2019 12:37:12 -0700 Subject: [PATCH 2/4] nl: fix integer-overflow bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Problem reported by Roland Illig (Bug#37585) * src/nl.c (print_lineno): Don’t rely on undefined behavior when checking for integer overflow. --- src/nl.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/nl.c b/src/nl.c index 43092b4fe..d85408c8c 100644 --- a/src/nl.c +++ b/src/nl.c @@ -275,14 +275,10 @@ build_type_arg (char const **typep, static void print_lineno (void) { - intmax_t next_line_no; - printf (lineno_format, lineno_width, line_no, separator_str); - next_line_no = line_no + page_incr; - if (next_line_no < line_no) + if (INT_ADD_WRAPV (line_no, page_incr, &line_no)) die (EXIT_FAILURE, 0, _("line number overflow")); - line_no = next_line_no; } /* Switch to a header section. */ -- 2.21.0
>From 72a348cc2d6160aa24bca93c23b1a17ffb5b1366 Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Thu, 3 Oct 2019 12:38:15 -0700 Subject: [PATCH 3/4] numfmt: avoid unlikely integer overflow MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/numfmt.c (parse_format_string): Report overflow if pad < -LONG_MAX, since that can’t be negated. --- src/numfmt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/numfmt.c b/src/numfmt.c index 305a88603..c56641cfd 100644 --- a/src/numfmt.c +++ b/src/numfmt.c @@ -1081,7 +1081,7 @@ parse_format_string (char const *fmt) errno = 0; pad = strtol (fmt + i, &endptr, 10); - if (errno == ERANGE) + if (errno == ERANGE || pad < -LONG_MAX) die (EXIT_FAILURE, 0, _("invalid format %s (width overflow)"), quote (fmt)); -- 2.21.0
>From d267ba04a6b4ad43e5a1311885f8ad9685502a5e Mon Sep 17 00:00:00 2001 From: Paul Eggert <[email protected]> Date: Thu, 3 Oct 2019 12:41:22 -0700 Subject: [PATCH 4/4] truncate: avoid integer-overflow assumptions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * src/truncate.c (do_ftruncate): Simplify overflow checking, and don’t rely on theoretically-nonportable assumptions like assuming that OFF_MAX < UINTMAX_MAX. --- src/truncate.c | 49 +++++++++++++++++++------------------------------ 1 file changed, 19 insertions(+), 30 deletions(-) diff --git a/src/truncate.c b/src/truncate.c index 4494ab51a..e7fb8543a 100644 --- a/src/truncate.c +++ b/src/truncate.c @@ -116,31 +116,29 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize, } if (block_mode) { - off_t const blksize = ST_BLKSIZE (sb); - if (ssize < OFF_T_MIN / blksize || ssize > OFF_T_MAX / blksize) + ptrdiff_t blksize = ST_BLKSIZE (sb); + intmax_t ssize0 = ssize; + if (INT_MULTIPLY_WRAPV (ssize, blksize, &ssize)) { error (0, 0, _("overflow in %" PRIdMAX - " * %" PRIdMAX " byte blocks for file %s"), - (intmax_t) ssize, (intmax_t) blksize, - quoteaf (fname)); + " * %" PRIdPTR " byte blocks for file %s"), + ssize0, blksize, quoteaf (fname)); return false; } - ssize *= blksize; } if (rel_mode) { - uintmax_t fsize; + off_t fsize; if (0 <= rsize) fsize = rsize; else { - off_t file_size; if (usable_st_size (&sb)) { - file_size = sb.st_size; - if (file_size < 0) + fsize = sb.st_size; + if (fsize < 0) { /* Sanity check. Overflow is the only reason I can think this would ever go negative. */ @@ -151,46 +149,37 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize, } else { - file_size = lseek (fd, 0, SEEK_END); - if (file_size < 0) + fsize = lseek (fd, 0, SEEK_END); + if (fsize < 0) { error (0, errno, _("cannot get the size of %s"), quoteaf (fname)); return false; } } - fsize = file_size; } if (rel_mode == rm_min) - nsize = MAX (fsize, (uintmax_t) ssize); + nsize = MAX (fsize, ssize); else if (rel_mode == rm_max) - nsize = MIN (fsize, (uintmax_t) ssize); + nsize = MIN (fsize, ssize); else if (rel_mode == rm_rdn) /* 0..ssize-1 -> 0 */ - nsize = (fsize / ssize) * ssize; - else if (rel_mode == rm_rup) - /* 1..ssize -> ssize */ + nsize = fsize - fsize % ssize; + else { - /* Here ssize>=1 && fsize>=0 */ - uintmax_t const overflow = ((fsize + ssize - 1) / ssize) * ssize; - if (overflow > OFF_T_MAX) + if (rel_mode == rm_rup) { - error (0, 0, _("overflow rounding up size of file %s"), - quoteaf (fname)); - return false; + /* 1..ssize -> ssize */ + off_t r = fsize % ssize; + ssize = r == 0 ? 0 : ssize - r; } - nsize = overflow; - } - else - { - if (ssize > OFF_T_MAX - (off_t)fsize) + if (INT_ADD_WRAPV (fsize, ssize, &nsize)) { error (0, 0, _("overflow extending size of file %s"), quoteaf (fname)); return false; } - nsize = fsize + ssize; } } else -- 2.21.0
