hashes: Fix Hkdf::expand panic at max length
What changed, and why it matters
This commit fixes a bug in the HKDF key-derivation function where requesting the maximum allowed output length could cause the program to panic (crash) due to a counter overflow. The fix replaces an 8-bit counter that could wrap around past 255 with a loop using a larger integer type, so the maximum-length expansion completes safely.
Upgrade to a version of rust-bitcoin containing this commit. If HKDF output lengths are externally supplied, validate them and ensure the fix is applied. No other immediate mitigation is required because the panic only occurs at the exact maximum allowed length.
Security signals we found
Integer overflow in loop counter leading to panic
Out-of-bounds memory access via overflowed index
Denial-of-service vector: attacker-controlled HKDF output length can trigger crash
RFC-5869 maximum output length boundary condition not handled safely
Evidence from the diff
In Hkdf::expand, the output buffer is allowed to be up to 255 * T::Hash::LEN bytes, requiring up to 255 hash blocks. The previous code used a u8 counter starting at 1 and incremented it each iteration. When total_blocks == 255, the loop condition counter <= 255 is true, the final block is processed, then counter += 1 overflows u8 back to 0, causing the loop to continue indefinitely and panic on out-of-bounds indexing. The patch changes the counter to a usize for loop (1..=total_blocks) and casts to u8 only when appending the counter byte to the HMAC input, eliminating the overflow.
Changed components
hashes/src/hkdf/mod.rsHkdf::expand methodInspect captured patch +9 / −12
### hashes/src/hkdf/mod.rs
@@ -71,36 +71,33 @@ where
return Err(MaxLengthError { max: MAX_OUTPUT_BLOCKS * T::Hash::LEN });
}
- // Counter starts at "1" based on RFC5869 spec and is committed to in the hash.
- let mut counter = 1u8;
// Ceiling calculation for the total number of blocks (iterations) required for the expand.
let total_blocks = okm.len().div_ceil(T::Hash::LEN);
- while counter <= total_blocks as u8 {
+ // Counter starts at "1" based on RFC5869 spec and is committed to in the hash.
+ for counter in 1..=total_blocks {
let mut engine: HmacEngine<T> = HmacEngine::new(self.prk.as_ref());
// First block does not have a previous block,
// all other blocks include last block in the HMAC input.
- if counter != 1u8 {
- let previous_start_index = (counter as usize - 2) * T::Hash::LEN;
- let previous_end_index = (counter as usize - 1) * T::Hash::LEN;
+ if counter != 1 {
+ let previous_start_index = (counter - 2) * T::Hash::LEN;
+ let previous_end_index = (counter - 1) * T::Hash::LEN;
engine.input(&okm[previous_start_index..previous_end_index]);
}
engine.input(info);
- engine.input(&[counter]);
+ engine.input(&[counter as u8]);
let t = engine.finalize();
- let start_index = (counter as usize - 1) * T::Hash::LEN;
+ let start_index = (counter - 1) * T::Hash::LEN;
// Last block might not take full hash length.
- let end_index = if counter == (total_blocks as u8) {
+ let end_index = if counter == total_blocks {
okm.len()
} else {
- counter as usize * T::Hash::LEN
+ counter * T::Hash::LEN
};
okm[start_index..end_index].copy_from_slice(&t.as_ref()[0..(end_index - start_index)]);
-
- counter += 1;
}
Ok(())Why this scored 62/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.