From: Pádraig Brady Date: Tue, 23 Sep 2025 14:38:51 +0000 (+0100) Subject: basenc: --base58: fix buffer overflow with input > 15MB X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=2ef53e5b0477f9d9361a11a471d704a96b1c99b8;p=thirdparty%2Fcoreutils.git basenc: --base58: fix buffer overflow with input > 15MB base58_length() operated naively on an int which resulted in an overflow to a negative number for any input > 2^31-1/138, i.e. 15,561,475 bytes. * src/basenc.c (base_length): Change input and output parameter types from int to idx_t since this needs to cater for the full input size in the base58 case. (base58_length): Likewise. Also reorder the calculation to be less exact, but doing the division first to minimize the chance of overflow (which now on 64 bit would only happen for inputs > around 6 Exa bytes). * tests/basenc/basenc-large.sh: Add a new test, that triggers with valgrind or ASAN. * tests/local.mk: Reference the new test. * NEWS: Mention the bug fix. --- diff --git a/NEWS b/NEWS index b159b47584..7a1a73113e 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,11 @@ GNU coreutils NEWS -*- outline -*- * Noteworthy changes in release ?.? (????-??-??) [?] +** Bug fixes + + `basenc --base58` would not operate correctly with input > 15561475 bytes. + [bug introduced with --base58 in coreutils-9.8] + * Noteworthy changes in release 9.8 (2025-09-22) [stable] diff --git a/src/basenc.c b/src/basenc.c index 1fb7a16f58..ae55f8e324 100644 --- a/src/basenc.c +++ b/src/basenc.c @@ -253,7 +253,7 @@ static_assert (DEC_BLOCKSIZE % 12 == 0); /* Complete encoded blocks are used. */ static_assert (DEC_BLOCKSIZE % 40 == 0); /* complete encoded blocks for base32*/ static_assert (DEC_BLOCKSIZE % 12 == 0); /* complete encoded blocks for base64*/ -static int (*base_length) (int i); +static idx_t (*base_length) (idx_t len); static int (*required_padding) (int i); static bool (*isubase) (unsigned char ch); static void (*base_encode) (char const *restrict in, idx_t inlen, @@ -427,8 +427,8 @@ decode_ctx_finalize (struct base_decode_context *ctx, #if BASE_TYPE == 42 -static int -base64_length_wrapper (int len) +static idx_t +base64_length_wrapper (idx_t len) { return BASE64_LENGTH (len); } @@ -526,8 +526,8 @@ base64url_decode_ctx_wrapper (struct base_decode_context *ctx, -static int -base32_length_wrapper (int len) +static idx_t +base32_length_wrapper (idx_t len) { return BASE32_LENGTH (len); } @@ -740,8 +740,8 @@ isubase16 (unsigned char ch) return ch < sizeof base16_to_int && 0 <= base16_to_int[ch]; } -static int -base16_length (int len) +static idx_t +base16_length (idx_t len) { return len * 2; } @@ -820,13 +820,14 @@ base16_decode_ctx (struct base_decode_context *ctx, - -static int -z85_length (int len) +ATTRIBUTE_PURE +static idx_t +z85_length (idx_t len) { /* Z85 does not allow padding, so no need to round to highest integer. */ - int outlen = (len * 5) / 4; - return outlen; + idx_t z85_len = (len * 5) / 4; + affirm (0 <= z85_len); + return z85_len; } static bool @@ -1015,8 +1016,8 @@ isubase2 (unsigned char ch) return ch == '0' || ch == '1'; } -static int -base2_length (int len) +static idx_t +base2_length (idx_t len) { return len * 8; } @@ -1206,12 +1207,17 @@ isubase58 (unsigned char ch) } -static int -base58_length (int len) +ATTRIBUTE_PURE +static idx_t +base58_length (idx_t len) { /* Base58 output length is approximately log(256)/log(58), - so ensure we've enough place for that + NUL. */ - return (len * 138) / 100 + 1; + which is approximately len * 138 / 100, + which is at most ((len + 100 - 1) / 100) * 138 + +1 to ensure we've enough place for NUL */ + idx_t base58_len = ((len + 99) / 100) * 138 + 1; + affirm (0 < base58_len); + return base58_len; } @@ -1268,6 +1274,7 @@ base58_encode (char const* data, size_t data_len, if (data_len - zeros) { mpz_import (num, data_len - zeros, 1, 1, 0, 0, data + zeros); + affirm (mpz_sizeinbase (num, 58) + 1 <= *outlen); for (p = mpz_get_str (p, 58, num); *p; p++) *p = gmp_to_base58[to_uchar (*p)]; } diff --git a/tests/basenc/basenc-large.sh b/tests/basenc/basenc-large.sh new file mode 100755 index 0000000000..e04db69d87 --- /dev/null +++ b/tests/basenc/basenc-large.sh @@ -0,0 +1,27 @@ +#!/bin/sh +# Test large inputs to basenc + +# Copyright (C) 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_ basenc + +# coreutils v9.8 would not operate correctly with > 15,561,475 bytes +truncate -s20MiB file.zeros || framework_failure_ + +test $(basenc --base58 file.zeros | wc -c) = 21247462 || fail=1 + +Exit $fail diff --git a/tests/local.mk b/tests/local.mk index 4aa199a197..ff5727f745 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -297,6 +297,7 @@ all_tests = \ tests/misc/basename.pl \ tests/basenc/base64.pl \ tests/basenc/basenc.pl \ + tests/basenc/basenc-large.sh \ tests/misc/close-stdout.sh \ tests/chroot/chroot-fail.sh \ tests/cksum/cksum.sh \