From: Paul Eggert Date: Thu, 26 Jun 2025 06:22:37 +0000 (-0700) Subject: od: fix some unlikely integer overflows X-Git-Tag: v9.8~278 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=88f30ee0a5c355701914d4446dc7ec729a344fa2;p=thirdparty%2Fcoreutils.git od: fix some unlikely integer overflows * src/od.c (print_n_spaces, pad_at, pad_at_overflow): New static functions. (struct tspec, PRINT_FIELDS, print_named_ascii, print_ascii) (decode_one_format, write_block, main): Use idx_t, not int, for counts that depend on the number of bytes in an object. (decode_one_format): Use print_n_spaces to output spaces. (PRINT_FIELDS, print_named_ascii, print_ascii): Use pad_at to avoid integer overflow. (write_block): Do not use %*s to pad, as the total pad might exceed INT_MAX. Instead, pad by hand with putchar (' '). (main): Use pad_at_overflow to report integer overflow due to oversize -w. Use better way to tell whether -w is used, without needing IF_LINT. * tests/od/big-w.sh: New test. * tests/local.mk (all_tests): Add it. --- diff --git a/NEWS b/NEWS index 116dd993ef..4a958770c7 100644 --- a/NEWS +++ b/NEWS @@ -20,6 +20,10 @@ GNU coreutils NEWS -*- outline -*- Previously it would have aborted. [bug introduced in coreutils-9.3] + od -w no longer silently mishandles enormous widths like 3037000500. + Instead, it either outputs correctly or diagnoses a too-large width. + [This bug was present in "the beginning".] + sort with key character offsets of SIZE_MAX, could induce a read of 1 byte before an allocated heap buffer. For example: 'sort +0.18446744073709551615R input' on 64 bit systems. diff --git a/src/od.c b/src/od.c index f385d6ce54..8bb463ca8b 100644 --- a/src/od.c +++ b/src/od.c @@ -124,11 +124,11 @@ struct tspec leading space, and PAD is total pad to divide among FIELDS. PAD is at least as large as FIELDS. */ void (*print_function) (idx_t fields, idx_t blank, void const *data, - char const *fmt, int width, int pad); + char const *fmt, int width, idx_t pad); char fmt_string[FMT_BYTES_ALLOCATED]; /* Of the style "%*d". */ bool hexl_mode_trailer; int field_width; /* Minimum width of a field, excluding leading space. */ - int pad_width; /* Total padding to be divided among fields. */ + idx_t pad_width; /* Total padding to be divided among fields. */ }; /* Convert the number of 8-bit bytes of a binary representation to @@ -450,16 +450,44 @@ Binary prefixes can be used, too: KiB=K, MiB=M, and so on.\n\ /* Define the print functions. */ +/* Print N spaces, where 0 <= N. + Do not rely on printf ("%*s", N, "") as N may exceed INT_MAX. */ +static void +print_n_spaces (intmax_t n) +{ + for (; 0 < n; n--) + putchar (' '); +} + +/* If there are FIELDS fields, return the total padding up to the + start of field I, where I < FIELDS. PAD is the total padding for + all fields. The result equals (PAD * I) / FIELDS, except it does + not suffer from internal overflow. */ +static idx_t +pad_at (idx_t fields, idx_t i, idx_t pad) +{ + /* This implementation assumes that (FIELDS - 1)^2 does not overflow + intmax_t, an assumption checked by pad_at_overflow. */ + intmax_t m = pad % fields; + return pad / fields * i + m * i / fields; +} +static bool +pad_at_overflow (idx_t fields) +{ + intmax_t product; + return ckd_mul (&product, fields - 1, fields - 1); +} + #define PRINT_FIELDS(N, T, FMT_STRING_DECL, ACTION) \ static void \ N (idx_t fields, idx_t blank, void const *block, \ - FMT_STRING_DECL, int width, int pad) \ + FMT_STRING_DECL, int width, idx_t pad) \ { \ T const *p = block; \ - int pad_remaining = pad; \ + idx_t pad_remaining = pad; \ for (idx_t i = fields; blank < i; i--) \ { \ - int next_pad = pad * (i - 1) / fields; \ + idx_t next_pad = pad_at (fields, i - 1, pad); \ int adjusted_width = pad_remaining - next_pad + width; \ T x; \ if (input_swap && sizeof (T) > 1) \ @@ -523,13 +551,12 @@ dump_hexl_mode_trailer (idx_t n_bytes, char const *block) static void print_named_ascii (idx_t fields, idx_t blank, void const *block, MAYBE_UNUSED char const *unused_fmt_string, - int width, int pad) + int width, idx_t pad) { unsigned char const *p = block; - int pad_remaining = pad; + idx_t pad_remaining = pad; for (idx_t i = fields; blank < i; i--) { - int next_pad = pad * (i - 1) / fields; int masked_c = *p++ & 0x7f; char const *s; char buf[2]; @@ -545,7 +572,9 @@ print_named_ascii (idx_t fields, idx_t blank, void const *block, s = buf; } - xprintf ("%*s", pad_remaining - next_pad + width, s); + idx_t next_pad = pad_at (fields, i - 1, pad); + int adjusted_width = pad_remaining - next_pad + width; + xprintf ("%*s", adjusted_width, s); pad_remaining = next_pad; } } @@ -553,13 +582,12 @@ print_named_ascii (idx_t fields, idx_t blank, void const *block, static void print_ascii (idx_t fields, idx_t blank, void const *block, MAYBE_UNUSED char const *unused_fmt_string, int width, - int pad) + idx_t pad) { unsigned char const *p = block; - int pad_remaining = pad; + idx_t pad_remaining = pad; for (idx_t i = fields; blank < i; i--) { - int next_pad = pad * (i - 1) / fields; unsigned char c = *p++; char const *s; char buf[4]; @@ -603,7 +631,9 @@ print_ascii (idx_t fields, idx_t blank, void const *block, s = buf; } - xprintf ("%*s", pad_remaining - next_pad + width, s); + idx_t next_pad = pad_at (fields, i - 1, pad); + int adjusted_width = pad_remaining - next_pad + width; + xprintf ("%*s", adjusted_width, s); pad_remaining = next_pad; } } @@ -656,7 +686,7 @@ decode_one_format (char const *s_orig, char const *s, char const **next, int size; enum output_format fmt; void (*print_function) (idx_t, idx_t, void const *, char const *, - int, int); + int, idx_t); char const *p; char c; int field_width; @@ -1253,22 +1283,24 @@ write_block (uintmax_t current_offset, idx_t n_bytes, for (idx_t i = 0; i < n_specs; i++) { int datum_width = width_bytes[spec[i].size]; - int fields_per_block = bytes_per_block / datum_width; - int blank_fields = (bytes_per_block - n_bytes) / datum_width; + idx_t fields_per_block = bytes_per_block / datum_width; + idx_t blank_fields = (bytes_per_block - n_bytes) / datum_width; if (i == 0) format_address (current_offset, '\0'); else - printf ("%*s", address_pad_len, ""); + print_n_spaces (address_pad_len); (*spec[i].print_function) (fields_per_block, blank_fields, curr_block, spec[i].fmt_string, spec[i].field_width, spec[i].pad_width); if (spec[i].hexl_mode_trailer) { - /* space-pad out to full line width, then dump the trailer */ + /* Space-pad out to full line width, then dump the trailer. */ int field_width = spec[i].field_width; - int pad_width = (spec[i].pad_width * blank_fields - / fields_per_block); - printf ("%*s", blank_fields * field_width + pad_width, ""); + for (idx_t f = 0; f < blank_fields; f++) + print_n_spaces (field_width); + idx_t pad_width = pad_at (fields_per_block, blank_fields, + spec[i].pad_width); + print_n_spaces (pad_width); dump_hexl_mode_trailer (n_bytes, curr_block); } putchar ('\n'); @@ -1594,9 +1626,8 @@ main (int argc, char **argv) { int n_files; int l_c_m; - idx_t desired_width IF_LINT ( = 0); + idx_t desired_width = 0; bool modern = false; - bool width_specified = false; bool ok = true; idx_t width_per_block = 0; static char const multipliers[] = "bEGKkMmPQRTYZ0"; @@ -1792,7 +1823,6 @@ main (int argc, char **argv) case 'w': modern = true; - width_specified = true; if (optarg == nullptr) { desired_width = 32; @@ -1958,9 +1988,9 @@ main (int argc, char **argv) /* Compute output block length. */ l_c_m = get_lcm (); - if (width_specified) + if (desired_width != 0) { - if (desired_width != 0 && desired_width % l_c_m == 0) + if (desired_width % l_c_m == 0) bytes_per_block = desired_width; else { @@ -1980,23 +2010,25 @@ main (int argc, char **argv) /* Compute padding necessary to align output block. */ for (idx_t i = 0; i < n_specs; i++) { - int fields_per_block = bytes_per_block / width_bytes[spec[i].size]; - int block_width = (spec[i].field_width + 1) * fields_per_block; + idx_t fields_per_block = bytes_per_block / width_bytes[spec[i].size]; + if (pad_at_overflow (fields_per_block)) + error (EXIT_FAILURE, 0, _("%td is too large"), desired_width); + idx_t block_width = (spec[i].field_width + 1) * fields_per_block; if (width_per_block < block_width) width_per_block = block_width; } for (idx_t i = 0; i < n_specs; i++) { - int fields_per_block = bytes_per_block / width_bytes[spec[i].size]; - int block_width = spec[i].field_width * fields_per_block; + idx_t fields_per_block = bytes_per_block / width_bytes[spec[i].size]; + idx_t block_width = spec[i].field_width * fields_per_block; spec[i].pad_width = width_per_block - block_width; } #ifdef DEBUG - printf ("lcm=%d, width_per_block=%zu\n", l_c_m, width_per_block); + printf ("lcm=%d, width_per_block=%td\n", l_c_m, width_per_block); for (idx_t i = 0; i < n_specs; i++) { - int fields_per_block = bytes_per_block / width_bytes[spec[i].size]; + idx_t fields_per_block = bytes_per_block / width_bytes[spec[i].size]; affirm (bytes_per_block % width_bytes[spec[i].size] == 0); affirm (1 <= spec[i].pad_width / fields_per_block); printf ("%d: fmt=\"%s\" in_width=%d out_width=%d pad=%d\n", diff --git a/tests/local.mk b/tests/local.mk index b68df41f76..03114f7592 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -269,6 +269,7 @@ all_tests = \ tests/tail/overlay-headers.sh \ tests/tail/pid.sh \ tests/tail/pid-pipe.sh \ + tests/od/big-w.sh \ tests/od/od.pl \ tests/od/od-endian.sh \ tests/od/od-float.sh \ diff --git a/tests/od/big-w.sh b/tests/od/big-w.sh new file mode 100755 index 0000000000..27c125c693 --- /dev/null +++ b/tests/od/big-w.sh @@ -0,0 +1,43 @@ +#!/bin/sh +# Check whether od -wN works with big N + +# Copyright 2025 Free Software Foundation, Inc. + +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. + +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. + +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . + +. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src +print_ver_ od +very_expensive_ + +export LC_ALL=C + +cat >exp <<'EOF' || framework_failure_ +0000000 x >x< +0000001 +EOF + +# Try values near sqrt(2**31) and sqrt(2**63). +for w in 46340 46341 3037000500 3037000501; do + printf x | od -w$w -tcz 2>err | tr -s ' ' ' ' >out + if test -s err; then + test ! -s out || fail=1 + else + compare exp out || fail=1 + outbytes=$(printf x | od -w$w -tcz | wc -c) + expbytes=$((4*$w + 21)) + test $expbytes -eq $outbytes || fail=1 + fi +done + +Exit $fail