Skip to content

Commit 82d3051

Browse files
oxarbitrageupbqdn
authored andcommitted
fix(network): enforce 160-entry protocol cap in read_headers (GHSA-438q-jx8f-cccv)
read_headers() relied on TrustedPreallocate which allows up to 1,409 CountedHeader entries per message — ~8.8x the Zcash protocol limit of 160. Unauthenticated peers could force excessive preallocation and parse work by sending oversized headers messages. Deserialize the CompactSize count explicitly and reject messages exceeding MAX_HEADERS_PER_MESSAGE (160) before parsing any entries. The new constant lives in zebra-chain so both zebra-network (receive) and zebra-state (send) can reference the same protocol value.
1 parent 273185f commit 82d3051

4 files changed

Lines changed: 58 additions & 28 deletions

File tree

zebra-chain/src/serialization.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,6 @@ pub use zcash_deserialize::{
4040
};
4141
pub use zcash_serialize::{
4242
zcash_serialize_bytes, zcash_serialize_bytes_external_count, zcash_serialize_empty_list,
43-
zcash_serialize_external_count, FakeWriter, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN,
43+
zcash_serialize_external_count, FakeWriter, ZcashSerialize, MAX_HEADERS_PER_MESSAGE,
44+
MAX_PROTOCOL_MESSAGE_LEN,
4445
};

zebra-chain/src/serialization/zcash_serialize.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ use super::{AtLeastOne, CompactSizeMessage};
99
/// This value is used to calculate safe preallocation limits for some types
1010
pub const MAX_PROTOCOL_MESSAGE_LEN: usize = 2 * 1024 * 1024;
1111

12+
/// The maximum number of block headers in a single `headers` protocol message.
13+
///
14+
/// <https://zips.z.cash/protocol/protocol.pdf#page=108>
15+
pub const MAX_HEADERS_PER_MESSAGE: usize = 160;
16+
1217
/// Consensus-critical serialization for Zcash.
1318
///
1419
/// This trait provides a generic serialization for consensus-critical

zebra-network/src/protocol/external/codec.rs

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@ use zebra_chain::{
1515
block::{self, Block},
1616
parameters::{Magic, Network},
1717
serialization::{
18-
sha256d, zcash_deserialize_bytes_external_count, zcash_deserialize_string_external_count,
19-
CompactSizeMessage, FakeWriter, ReadZcashExt, SerializationError as Error,
20-
ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN,
18+
sha256d, zcash_deserialize_bytes_external_count, zcash_deserialize_external_count,
19+
zcash_deserialize_string_external_count, CompactSizeMessage, FakeWriter, ReadZcashExt,
20+
SerializationError as Error, ZcashDeserialize, ZcashDeserializeInto, ZcashSerialize,
21+
MAX_HEADERS_PER_MESSAGE, MAX_PROTOCOL_MESSAGE_LEN,
2122
},
2223
transaction::Transaction,
2324
};
@@ -678,7 +679,19 @@ impl Codec {
678679
///
679680
/// [Zcash block header](https://zips.z.cash/protocol/protocol.pdf#page=84)
680681
fn read_headers<R: Read>(&self, mut reader: R) -> Result<Message, Error> {
681-
Ok(Message::Headers(Vec::zcash_deserialize(&mut reader)?))
682+
// CompactSizeMessage is bounded to MAX_PROTOCOL_MESSAGE_LEN on deserialization.
683+
let count: CompactSizeMessage = (&mut reader).zcash_deserialize_into()?;
684+
// Infallible: CompactSizeMessage wraps u32, which always fits in usize.
685+
let count: usize = count.into();
686+
if count > MAX_HEADERS_PER_MESSAGE {
687+
return Err(Error::Parse(
688+
"headers message exceeds the protocol limit of 160 entries",
689+
));
690+
}
691+
Ok(Message::Headers(zcash_deserialize_external_count(
692+
count,
693+
&mut reader,
694+
)?))
682695
}
683696

684697
fn read_getheaders<R: Read>(&self, mut reader: R) -> Result<Message, Error> {

zebra-network/src/protocol/external/codec/tests/vectors.rs

Lines changed: 34 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -588,55 +588,66 @@ fn reject_command_and_reason_size_limits() {
588588
}
589589
}
590590

591-
/// Reproduce GHSA-438q-jx8f-cccv: read_headers() accepts more than the
592-
/// protocol-maximum 160 headers because the receive path relies on
593-
/// TrustedPreallocate (ceiling ≈ 1 409) instead of MAX_FIND_BLOCK_HEADERS_RESULTS.
591+
/// Regression test for GHSA-438q-jx8f-cccv: read_headers() must reject
592+
/// inbound `headers` messages with more than 160 entries.
594593
#[test]
595-
fn headers_message_exceeding_protocol_cap_is_accepted() {
594+
fn headers_message_exceeding_protocol_cap_is_rejected() {
596595
use zebra_chain::serialization::ZcashDeserializeInto;
597596

598597
let _init_guard = zebra_test::init();
599598

600-
// Parse the dummy header from test vectors.
601599
let header: block::Header = zebra_test::vectors::DUMMY_HEADER
602600
.zcash_deserialize_into()
603601
.expect("dummy header should deserialize");
604602
let counted = block::CountedHeader {
605603
header: header.into(),
606604
};
607605

608-
// 161 headers — one more than the Zcash protocol limit of 160.
609-
let count = 161;
610-
let msg = Message::Headers(vec![counted; count]);
606+
// 161 headers — one more than the protocol limit of 160.
607+
let msg = Message::Headers(vec![counted.clone(); 161]);
611608

612-
// Encode via the codec (no cap on the send side either).
613609
let mut codec = Codec::builder().finish();
614610
let mut bytes = BytesMut::new();
615611
codec
616612
.encode(msg, &mut bytes)
617-
.expect("encoding 161 headers should succeed");
613+
.expect("encoding should succeed");
614+
615+
codec
616+
.decode(&mut bytes)
617+
.expect_err("decoding 161 headers should be rejected");
618+
}
619+
620+
/// Verify that a headers message at exactly the protocol cap (160) is accepted.
621+
#[test]
622+
fn headers_message_at_protocol_cap_is_accepted() {
623+
use zebra_chain::serialization::ZcashDeserializeInto;
624+
625+
let _init_guard = zebra_test::init();
626+
627+
let header: block::Header = zebra_test::vectors::DUMMY_HEADER
628+
.zcash_deserialize_into()
629+
.expect("dummy header should deserialize");
630+
let counted = block::CountedHeader {
631+
header: header.into(),
632+
};
633+
634+
let msg = Message::Headers(vec![counted; 160]);
635+
636+
let mut codec = Codec::builder().finish();
637+
let mut bytes = BytesMut::new();
638+
codec
639+
.encode(msg, &mut bytes)
640+
.expect("encoding should succeed");
618641

619-
// Decode — this succeeds because read_headers() has no 160-entry cap.
620642
let decoded = codec
621643
.decode(&mut bytes)
622644
.expect("decoding should not error")
623645
.expect("a message should be present");
624646

625647
match decoded {
626-
Message::Headers(headers) => {
627-
assert_eq!(
628-
headers.len(),
629-
count,
630-
"codec accepted {count} headers; protocol cap is 160"
631-
);
632-
}
648+
Message::Headers(headers) => assert_eq!(headers.len(), 160),
633649
other => panic!("expected Headers, got {other:?}"),
634650
}
635-
636-
// Show the TrustedPreallocate ceiling that enables the amplification.
637-
use zebra_chain::serialization::TrustedPreallocate;
638-
let max = block::CountedHeader::max_allocation();
639-
assert_eq!(max, 1409, "preallocation ceiling should be ~8.8x the protocol cap of 160");
640651
}
641652

642653
/// Check that the version test vector deserialization fails when there's a network magic mismatch.

0 commit comments

Comments
 (0)