Merge rust-bitcoin/rust-bitcoin#6781: base58: Use `div_ceil` for reserve length calculation
What the developer wrote
Authored by Andrew Poelstra
Merge rust-bitcoin/rust-bitcoin#6781: base58: Use `div_ceil` for reserve length calculation
196c27161d1e6e4d76b4be5978a7c45a864c96a6 Add regression test for reserve length calculation (Mitchell Bagot)
5b3cc125fdb6d015e0ddf14306dd7810a9110552 base58: Use div_ceil for reserve length calculation (Mitchell Bagot)
Pull request description:
When determining the size needed for an encoding buffer, the code currently uses an approximation by multiplying the payload length by 1.37. However, as this is done with integer operations, the value is rounded down, which for some payload lengths (namely 94), the encoding can panic due to an undersized buffer.
Use div_ceil for reserve length calculation to prevent panic on 94 byte encode.
ACKs for top commit:
satsfy:
tACK 196c27161d1e6e4d76b4be5978a7c45a864c96a6
tcharding:
ACK 196c27161d1e6e4d76b4be5978a7c45a864c96a6
apoelstra:
ACK 196c27161d1e6e4d76b4be5978a7c45a864c96a6; successfully ran local tests
Tree-SHA512: d0e737115211799874efecb1229885ddefae941a9132df3e19ea1f40f6c0c0c5238d42224a194b215e9657cb649b80c8511e4540029b5499a6f70cd254f71866
What changed, and why it matters
This commit fixes a bug in the base58 encoding function used for Bitcoin-style addresses. When encoding certain payload sizes (specifically 94 bytes), the program could panic because it reserved a buffer that was slightly too small. The fix rounds the buffer-size calculation up instead of down, preventing the crash. The commit also adds a regression test using a 90-byte payload with checksum (which triggers the same rounding edge case).
No immediate action beyond applying the patch; users relying on base58 encoding of large or adversarially chosen payloads should update. Review whether any other length-estimation helpers in the crate use similar integer-division patterns.
Security signals we found
Integer truncation leading to undersized buffer allocation
Potential panic in encoding path (denial-of-service vector)
Regression test added for the edge-case payload length
Evidence from the diff
The encoded_reserve_len helper in base58/src/lib.rs estimated the encoded length as unencoded_len * 137 / 100. Because integer division truncates, the reserved capacity could be one byte short for some inputs, causing Base58CkString::encode_unbounded to panic when the final encoded string exceeded the pre-allocated buffer. The patch changes the calculation to (unencoded_len * 137).div_ceil(100), ensuring the buffer is always large enough. A regression test encoding 90 bytes of 0xFF (which with checksum hits the edge case) is added.
Changed components
base58/src/lib.rsencoded_reserve_lenBase58CkString::encode_unboundedInspect captured patch +5 / −1
### base58/src/lib.rs
@@ -446,7 +446,7 @@ impl fmt::Debug for Base58CkString {
#[cfg(feature = "alloc")]
const fn encoded_reserve_len(unencoded_len: usize) -> usize {
// log2(256) / log2(58) ~ 1.37 = 137 / 100
- unencoded_len * 137 / 100
+ (unencoded_len * 137).div_ceil(100)
}
/// Returns the length to reserve when encoding base58 with checksum
@@ -565,6 +565,10 @@ mod tests {
Base58CkString::encode_unbounded(&addr[..]).as_str(),
"1PfJpZsjreyVrqeoAfabrRwwjQyoSQMmHH"
);
+
+ let data = [0xFFu8; 90];
+ #[cfg(feature = "alloc")]
+ let _ = Base58CkString::encode_unbounded(&data);
}
#[test]Why this scored 46/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.