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 <noreply@anthropic.com>
This commit is contained in:
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user