Subject: basenc triggers undefined-behaviour in mini-gmp
From: Bruno Haible <[email-masked]>
To: [email-masked]
Date: Tue, 12 Aug 2025 02:34:55 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

The CI this week reports a new test failure of the tests/basenc/basenc test,
when compiled with sanitizers.

How to reproduce:
1. Build the current coreutils with
   CC="clang -fsanitize=address,undefined,signed-integer-overflow,shift,integer-divide-by-zero -fno-sanitize-recover=undefined"
   configuring it with option --without-libgmp . (There is no issue when
   coreutils uses the real gmp.)
2. 
$ src/basenc --base58 < /dev/null
../lib/mini-gmp.c:4529:9: runtime error: applying non-zero offset 18446744073709551615 to null pointer
SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../lib/mini-gmp.c:4529:9 
Aborted

gdb stack trace:

#7  0x0000555555696e79 in __ubsan_handle_pointer_overflow_abort ()
    at /home/runner/work/llvm-project/llvm-project/compiler-rt/lib/ubsan/ubsan_handlers.cpp:855
#8  0x00005555556e27a2 in mpz_import (r=0x7bfff5900060, count=0, order=1, size=1, endian=-1, nails=0, src=0x0) at ../lib/mini-gmp.c:4529
#9  0x00005555556a70a7 in base58_encode (data=0x0, data_len=0, out=0x7edff6de0400 '\276' <repeats 200 times>..., outlen=0x7bfff5b000c0)
    at ../src/basenc.c:1178
#10 0x00005555556a2d56 in base58_encode_ctx_finalize (ctx=0x7bfff5b00060, out=0x7bfff5b00040, outlen=0x7bfff5b000c0)
    at ../src/basenc.c:1200
#11 0x00005555556a65d1 in do_encode (in=0x7ffff7e038e0 <_IO_2_1_stdin_>, infile=0x55555570fd60 <str> "-", 
    out=0x7ffff7e045c0 <_IO_2_1_stdout_>, wrap_column=76) at ../src/basenc.c:1431
#12 0x0000555555699973 in main (argc=2, argv=0x7fffffffcfd8) at ../src/basenc.c:1736

Adding a non-zero offset to a NULL pointer is undefined behaviour per
ISO C 23 § 6.5.7.(9) as amended by N3322.

Should mpz_import accept count=0, src=NULL arguments? Hard to say from the gmp
documentation https://gmplib.org/manual/Integer-Import-and-Export .

If yes, then it's a bug in mini-gmp.c around line 4529.
If no, it's a bug in coreutils/src/basenc.c.

If yes, feel free to report that to the GMP people, without CCing me, please.
If no, feel free to apply the attached fix. It passes "make check".

Bruno

From 1d5042677fc8be29aa95451db2cd23b9ab3a32a8 Mon Sep 17 00:00:00 2001
From: Bruno Haible <[email-masked]>
Date: Tue, 12 Aug 2025 02:25:41 +0200
Subject: [PATCH] basenc: Don't trigger undefined behaviour in mini-gmp

* src/basenc.c (base58_encode): Avoid calling mpz_import on an empty
limb sequence.
---
 src/basenc.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/basenc.c b/src/basenc.c
index dbe3b388f..84789e2de 100644
--- a/src/basenc.c
+++ b/src/basenc.c
@@ -1175,10 +1175,12 @@ base58_encode (char const* data, size_t data_len,
   /* Use GMP to convert from base 256 to base 58.  */
   mpz_t num;
   mpz_init (num);
-  mpz_import (num, data_len - zeros, 1, 1, 0, 0, data + zeros);
   if (data_len - zeros)
-    for (p = mpz_get_str (p, 58, num); *p; p++)
-      *p = gmp_to_base58[to_uchar (*p)];
+    {
+      mpz_import (num, data_len - zeros, 1, 1, 0, 0, data + zeros);
+      for (p = mpz_get_str (p, 58, num); *p; p++)
+        *p = gmp_to_base58[to_uchar (*p)];
+    }
   mpz_clear (num);
 
   *outlen = p - out;
-- 
2.50.1