From c43f645f15b0526dbeb381ea312d57cee1502f0b Mon Sep 17 00:00:00 2001 From: Kgothatso Ngako Date: Mon, 31 Aug 2026 13:25:26 +0200 Subject: [PATCH] chilldkg: fix out-of-bounds write in recovery data parsing secp256k1_chilldkg_deserialize_recovery_data read the threshold t from the first four bytes of the recovery blob and used it directly as the loop bound writing into the fixed-size sum_coms[SECP256K1_CHILLDKG_MAX_PARTICIPANTS] array. params_validate would reject an out-of-range t, but it only runs after the parse -- the overflow happens first. The reference implementation is safe only because Python lists grow dynamically; the C port lost that implicit bound. Reachable with attacker-supplied input from both public entry points, secp256k1_chilldkg_participant_recover and secp256k1_chilldkg_coordinator_recover (recovery data is untrusted by design: participants who never received cmsg2 are expected to accept recovery data from third parties). Confirmed with AddressSanitizer: t = 129 with a 4261-byte blob writes one group element past the array; t = 30000 with a ~1 MB blob writes ~1.2 MB of attacker- controlled group elements past it. The spec's own "invalid threshold" recovery vector cannot catch this because the memory-safe reference never exercises a t large enough to overflow a fixed C array. Fix: reject t < 1 and t > SECP256K1_CHILLDKG_MAX_PARTICIPANTS right after reading t, before any use as a loop bound, and reject recovery_len > SECP256K1_CHILLDKG_MAX_RECOVERY_LEN up front for consistency with the recovery-ack paths (the macro moves next to the parser; with t and n bounded the length cap is redundant, but it documents and enforces the invariant at the single choke point). Invalid recovery data is a plain input error (RecoveryDataError in the reference), so this maps to SECP256K1_CHILLDKG_INVALID_INPUT with outputs zeroed, like any other malformed blob. --- src/modules/chilldkg/main_impl.h | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/src/modules/chilldkg/main_impl.h b/src/modules/chilldkg/main_impl.h index 310ce4c2..122911a4 100644 --- a/src/modules/chilldkg/main_impl.h +++ b/src/modules/chilldkg/main_impl.h @@ -703,6 +703,10 @@ size_t secp256k1_chilldkg_investigation_msg_len(size_t n_participants) { return 65 * n_participants; } +/* Maximum length of the recovery data: 4 + 33*t + 162*n bytes with + * t, n <= SECP256K1_CHILLDKG_MAX_PARTICIPANTS. */ +#define SECP256K1_CHILLDKG_MAX_RECOVERY_LEN (4 + 195 * SECP256K1_CHILLDKG_MAX_PARTICIPANTS) + /* Parse the self-delimiting recovery data (deserialize_recovery_data in * chilldkg_ref/chilldkg.py): * u32be(t) || sum_coms (33*t) || hostpubkeys (33*n) || pubnonces (33*n) || @@ -717,10 +721,16 @@ static int secp256k1_chilldkg_deserialize_recovery_data(uint32_t *t_out, size_t const unsigned char *ptr; int overflow; - if (recovery_len < 4) { + if (recovery_len < 4 || recovery_len > SECP256K1_CHILLDKG_MAX_RECOVERY_LEN) { return 0; } t = secp256k1_read_be32(recovery); + /* Bound t before using it as a loop bound over the fixed-size sum_coms + * array: the recovery data is untrusted input. (The reference gets this + * implicitly from Python's dynamically-sized lists.) */ + if (t < 1 || t > SECP256K1_CHILLDKG_MAX_PARTICIPANTS) { + return 0; + } rest_len = recovery_len - 4; if (rest_len < 33 * t || (rest_len - 33 * t) % 162 != 0) { return 0; @@ -920,10 +930,6 @@ static void secp256k1_chilldkg_recovery_ack_message(unsigned char *msg, const un memcpy(msg + 37, recovery, recovery_len); } -/* The maximum length of recovery data: 4 + 33*t + 162*n with t, n <= - * SECP256K1_CHILLDKG_MAX_PARTICIPANTS. */ -#define SECP256K1_CHILLDKG_MAX_RECOVERY_LEN (4 + 195 * SECP256K1_CHILLDKG_MAX_PARTICIPANTS) - /* Check that the recovery data parses and matches the given session * parameters (part of participant_recovery_ack_sign and * participant_recovery_acks_verify in the reference). Returns 1 on match, 0