Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions crates/chia-protocol/src/full_node_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ pub struct RequestBlocks {
pub struct RespondBlocks {
start_height: u32,
end_height: u32,
#[chia(max_length = 64)]
blocks: Vec<FullBlock>,
}
Comment thread
arvidn marked this conversation as resolved.

Expand Down
28 changes: 28 additions & 0 deletions crates/chia-protocol/src/wallet_protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,14 +72,17 @@ pub struct RejectHeaderRequest {
pub struct RequestRemovals {
height: u32,
header_hash: Bytes32,
#[chia(max_length = 30000)]
coin_names: Option<Vec<Bytes32>>,
}
Comment thread
arvidn marked this conversation as resolved.

#[streamable(message)]
pub struct RespondRemovals {
height: u32,
header_hash: Bytes32,
#[chia(max_length = 30000)]
coins: Vec<(Bytes32, Option<Coin>)>,
#[chia(max_length = 30000)]
proofs: Option<Vec<(Bytes32, Bytes)>>,
}

Expand All @@ -93,14 +96,17 @@ pub struct RejectRemovalsRequest {
pub struct RequestAdditions {
height: u32,
header_hash: Option<Bytes32>,
#[chia(max_length = 30000)]
puzzle_hashes: Option<Vec<Bytes32>>,
}

#[streamable(message)]
pub struct RespondAdditions {
height: u32,
header_hash: Bytes32,
#[chia(max_length = 30000)]
coins: Vec<(Bytes32, Vec<Coin>)>,
#[chia(max_length = 30000)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Vec size limits may break existing wallet clients

Medium Severity

The 30000/35000 caps on puzzle_hashes/coin_names in RequestRemovals, RequestAdditions, RequestPuzzleState, RespondPuzzleState, and CoinStateUpdate are far below the 1_600_000 used for RegisterForPhUpdates/RegisterForCoinUpdates. Wallets that currently operate with more puzzle hashes per request (each address counts as a hash) will see legitimate messages rejected with SequenceTooLarge after this change.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8c79e84. Configure here.

proofs: Option<Vec<(Bytes32, Bytes, Option<Bytes>)>>,
}

Expand All @@ -114,6 +120,7 @@ pub struct RejectAdditionsRequest {
pub struct RespondBlockHeaders {
start_height: u32,
end_height: u32,
#[chia(max_length = 128)]
header_blocks: Vec<HeaderBlock>,
}

Expand Down Expand Up @@ -146,32 +153,39 @@ pub struct RejectHeaderBlocks {
pub struct RespondHeaderBlocks {
start_height: u32,
end_height: u32,
#[chia(max_length = 64)]
header_blocks: Vec<HeaderBlock>,
}

#[streamable(message)]
pub struct RegisterForPhUpdates {
#[chia(max_length = 1600000)]
puzzle_hashes: Vec<Bytes32>,
min_height: u32,
}

#[streamable(message)]
pub struct RespondToPhUpdates {
#[chia(max_length = 1600000)]
puzzle_hashes: Vec<Bytes32>,
min_height: u32,
#[chia(max_length = 500000)]
coin_states: Vec<CoinState>,
}

#[streamable(message)]
pub struct RegisterForCoinUpdates {
#[chia(max_length = 1600000)]
coin_ids: Vec<Bytes32>,
min_height: u32,
}

#[streamable(message)]
pub struct RespondToCoinUpdates {
#[chia(max_length = 1600000)]
coin_ids: Vec<Bytes32>,
min_height: u32,
#[chia(max_length = 500000)]
coin_states: Vec<CoinState>,
}

Expand All @@ -180,6 +194,7 @@ pub struct CoinStateUpdate {
height: u32,
fork_height: u32,
peak_hash: Bytes32,
#[chia(max_length = 30000)]
items: Vec<CoinState>,
}

Expand All @@ -190,6 +205,7 @@ pub struct RequestChildren {

#[streamable(message)]
pub struct RespondChildren {
#[chia(max_length = 30000)]
coin_states: Vec<CoinState>,
}

Expand Down Expand Up @@ -217,21 +233,25 @@ pub struct RespondFeeEstimates {

#[streamable(message)]
pub struct RequestRemovePuzzleSubscriptions {
#[chia(max_length = 1600000)]
puzzle_hashes: Option<Vec<Bytes32>>,
}

#[streamable(message)]
pub struct RespondRemovePuzzleSubscriptions {
#[chia(max_length = 1600000)]
puzzle_hashes: Vec<Bytes32>,
}

#[streamable(message)]
pub struct RequestRemoveCoinSubscriptions {
#[chia(max_length = 1600000)]
coin_ids: Option<Vec<Bytes32>>,
}

#[streamable(message)]
pub struct RespondRemoveCoinSubscriptions {
#[chia(max_length = 1600000)]
coin_ids: Vec<Bytes32>,
}

Expand All @@ -245,6 +265,7 @@ pub struct CoinStateFilters {

#[streamable(message)]
pub struct RequestPuzzleState {
#[chia(max_length = 35000)]
puzzle_hashes: Vec<Bytes32>,
previous_height: Option<u32>,
header_hash: Bytes32,
Expand All @@ -254,10 +275,12 @@ pub struct RequestPuzzleState {

#[streamable(message)]
pub struct RespondPuzzleState {
#[chia(max_length = 33000)]
puzzle_hashes: Vec<Bytes32>,
height: u32,
header_hash: Bytes32,
is_finished: bool,
#[chia(max_length = 500000)]
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inconsistent puzzle hash limits between request and response

Medium Severity

RequestPuzzleState accepts up to 35000 puzzle hashes while RespondPuzzleState only allows 33000. Since the response echoes back the requested puzzle hashes, a peer accepting a request near the upper limit cannot generate a response that successfully parses on the other side, producing an unrecoverable protocol failure for otherwise valid requests.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8c79e84. Configure here.

coin_states: Vec<CoinState>,
}

Expand All @@ -268,6 +291,7 @@ pub struct RejectPuzzleState {

#[streamable(message)]
pub struct RequestCoinState {
#[chia(max_length = 500000)]
coin_ids: Vec<Bytes32>,
previous_height: Option<u32>,
header_hash: Bytes32,
Expand All @@ -276,7 +300,9 @@ pub struct RequestCoinState {

#[streamable(message)]
pub struct RespondCoinState {
#[chia(max_length = 500000)]
coin_ids: Vec<Bytes32>,
#[chia(max_length = 500000)]
coin_states: Vec<CoinState>,
}

Expand Down Expand Up @@ -334,11 +360,13 @@ pub struct RemovedMempoolItem {

#[streamable(message)]
pub struct MempoolItemsAdded {
#[chia(max_length = 30000)]
transaction_ids: Vec<Bytes32>,
}

#[streamable(message)]
pub struct MempoolItemsRemoved {
#[chia(max_length = 30000)]
removed_items: Vec<RemovedMempoolItem>,
}

Expand Down
85 changes: 74 additions & 11 deletions crates/chia-traits/src/streamable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,19 +124,28 @@ impl<T: Streamable> Streamable for Vec<T> {
}

fn parse<const TRUSTED: bool>(input: &mut Cursor<&[u8]>) -> Result<Self> {
let len = u32::parse::<TRUSTED>(input)?;
parse_vec_with_max_length::<T, TRUSTED>(input, u32::MAX as usize)
}
}

let mut ret = if mem::size_of::<T>() == 0 {
Vec::<T>::new()
} else {
let limit = 2 * 1024 * 1024 / mem::size_of::<T>();
Vec::<T>::with_capacity(std::cmp::min(limit, len as usize))
};
for _ in 0..len {
ret.push(T::parse::<TRUSTED>(input)?);
}
Ok(ret)
pub fn parse_vec_with_max_length<T: Streamable, const TRUSTED: bool>(
input: &mut Cursor<&[u8]>,
max_length: usize,
) -> Result<Vec<T>> {
let len = u32::parse::<TRUSTED>(input)?;
if (len as usize) > max_length {
return Err(Error::SequenceTooLarge);
}
let mut ret = if mem::size_of::<T>() == 0 {
Vec::<T>::new()
} else {
let limit = 2 * 1024 * 1024 / mem::size_of::<T>();
Vec::<T>::with_capacity(std::cmp::min(limit, len as usize))
};
Comment thread
Rigidity marked this conversation as resolved.
for _ in 0..len {
ret.push(T::parse::<TRUSTED>(input)?);
}
Ok(ret)
}
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
arvidn marked this conversation as resolved.

impl Streamable for String {
Expand Down Expand Up @@ -784,3 +793,57 @@ fn test_stream_enum() {
assert_eq!(stream::<TestEnum>(&TestEnum::B), &[1_u8]);
assert_eq!(stream::<TestEnum>(&TestEnum::C), &[255_u8]);
}

#[cfg(test)]
#[derive(Streamable, PartialEq, Debug)]
struct TestBoundedVec {
#[chia(max_length = 3)]
items: Vec<u32>,
}

#[test]
fn test_parse_bounded_vec() {
let buf: &[u8] = &[0, 0, 0, 3, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3];
from_bytes::<TestBoundedVec>(
buf,
TestBoundedVec {
items: vec![1, 2, 3],
},
);
}

#[test]
fn test_parse_bounded_vec_too_large() {
let buf: &[u8] = &[0, 0, 0, 4, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3, 0, 0, 0, 4];
from_bytes_fail::<TestBoundedVec>(buf, Error::SequenceTooLarge);
}

#[cfg(test)]
#[derive(Streamable, PartialEq, Debug)]
struct TestBoundedOptionVec {
#[chia(max_length = 2)]
items: Option<Vec<u32>>,
}

#[test]
fn test_parse_bounded_option_vec_none() {
let buf: &[u8] = &[0];
from_bytes::<TestBoundedOptionVec>(buf, TestBoundedOptionVec { items: None });
}

#[test]
fn test_parse_bounded_option_vec_some() {
let buf: &[u8] = &[1, 0, 0, 0, 2, 0, 0, 0, 1, 0, 0, 0, 2];
from_bytes::<TestBoundedOptionVec>(
buf,
TestBoundedOptionVec {
items: Some(vec![1, 2]),
},
);
}

#[test]
fn test_parse_bounded_option_vec_too_large() {
let buf: &[u8] = &[1, 0, 0, 0, 3, 0, 0, 0, 1, 0, 0, 0, 2, 0, 0, 0, 3];
from_bytes_fail::<TestBoundedOptionVec>(buf, Error::SequenceTooLarge);
}
Loading
Loading