From bf31d42486ddea9f2d9d2b55621a2e016c0411bb Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Tue, 1 Sep 2026 23:36:43 +0200 Subject: [PATCH] frost: fix stack buffer overflow in frost_nonce_test frost_nonce_test_internal declares extra_in as 16 bytes and filled it with testrand256, which writes 32. Three iterations of the loop each overflowed the array by 16 bytes. AddressSanitizer is unambiguous about it: ERROR: AddressSanitizer: stack-buffer-overflow WRITE of size 1 ... in testrand256 src/testrand_impl.h:90 #1 frost_nonce_test_internal src/modules/frost/tests_impl.h:533 [4336, 4352) 'extra_in' (line 510) <== Memory access at offset 4352 overflows this variable The plain test suite passes regardless, which is why this survived: under the current stack layout the 16 stray bytes land on secshare, which is re-derived into pubshare_tmp immediately afterwards, so the two stay consistent and nothing downstream notices. That is luck, not correctness -- the target is whatever the compiler happens to place next, and msglen, extra_in_len or i are equally plausible. This should have been failing CI already: the sanitizers_debian job in .github/workflows/ci.yml sets FROST: 'yes' alongside CFLAGS: '-fsanitize=undefined,address -g'. Use testrand_bytes_test with an explicit length, matching how the musig tests fill their own extra_input (src/modules/musig/tests_impl.h:578). extra_in_len is already drawn from testrand_int(sizeof(extra_in) + 1), so the buffer stays 16 bytes and the value range is unchanged. Scanned all three new test files for other fixed-width writers aimed at undersized arrays; this was the only one. With the fix, the frost, chilldkg and iceberg suites run clean under -fsanitize=address,undefined. Co-Authored-By: Claude Opus 5 --- src/modules/frost/tests_impl.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/modules/frost/tests_impl.h b/src/modules/frost/tests_impl.h index 16a7beb2..776f96b9 100644 --- a/src/modules/frost/tests_impl.h +++ b/src/modules/frost/tests_impl.h @@ -530,7 +530,7 @@ static void frost_nonce_test_internal(void) { msglen = testrand_int(sizeof(msg) + 1); testrand256(msg); extra_in_len = testrand_int(sizeof(extra_in) + 1); - testrand256(extra_in); + testrand_bytes_test(extra_in, sizeof(extra_in)); if (testrand_bits(1)) { secp256k1_pubkey pubshare_tmp; CHECK(secp256k1_ec_pubkey_create(CTX, &pubshare_tmp, secshare) == 1);