Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #2339 #2340

Merged
merged 4 commits into from
Jun 7, 2022
Merged
Show file tree
Hide file tree
Changes from 3 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
4 changes: 4 additions & 0 deletions bin/wasm-node/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Fixed

- Fix multiple panics when decoding network messages in case where these messages were truncated. ([#2340](https://github.com/paritytech/smoldot/pull/2340))

## 0.6.17 - 2022-05-31

### Changed
Expand Down
4 changes: 2 additions & 2 deletions src/libp2p/connection/noise.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,10 +746,10 @@ impl HandshakeInProgress {
let (identity_key, identity_sig) = {
let mut parser =
nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((
nom::combinator::complete(protobuf::message_decode((
protobuf::bytes_tag_decode(1),
protobuf::bytes_tag_decode(2),
)),
))),
);
match nom::Finish::finish(parser(&decoded_payload)) {
Ok((_, ((key,), (sig,)))) => (key, sig),
Expand Down
7 changes: 4 additions & 3 deletions src/libp2p/peer_id.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,17 @@ impl PublicKey {

// As indicated in the libp2p specification, the public key must be encoded
// deterministically, and thus the fields are decoded deterministically in a precise order.
let mut parser =
nom::combinator::all_consuming::<_, _, ErrorWrapper, _>(nom::sequence::tuple((
let mut parser = nom::combinator::all_consuming::<_, _, ErrorWrapper, _>(
nom::combinator::complete(nom::sequence::tuple((
nom::combinator::map_res(protobuf::enum_tag_decode(1), |val| match val {
0 | 1 | 2 | 3 => Ok(val),
_ => Err(FromProtobufEncodingError::UnknownAlgorithm),
}),
nom::combinator::map_res(protobuf::bytes_tag_decode(2), |d| {
<[u8; 32]>::try_from(d).map_err(|_| FromProtobufEncodingError::BadEd25519Key)
}),
)));
))),
);

match nom::Finish::finish(parser(bytes)) {
Ok((_, (1, key))) => Ok(PublicKey::Ed25519(key)),
Expand Down
4 changes: 2 additions & 2 deletions src/network/kademlia.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ pub fn decode_find_node_response(
response_bytes: &[u8],
) -> Result<Vec<(peer_id::PeerId, Vec<multiaddr::Multiaddr>)>, DecodeFindNodeResponseError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode::<((_,), Vec<_>), _, _>((
nom::combinator::complete(protobuf::message_decode::<((_,), Vec<_>), _, _>((
protobuf::enum_tag_decode(1),
protobuf::message_tag_decode(
8,
Expand All @@ -75,7 +75,7 @@ pub fn decode_find_node_response(
protobuf::bytes_tag_decode(2),
)),
),
)),
))),
);

let closer_peers: Vec<_> = match nom::Finish::finish(parser(response_bytes)) {
Expand Down
8 changes: 4 additions & 4 deletions src/network/protocol/block_announces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ pub fn encode_block_announce(
/// Decodes a block announcement.
pub fn decode_block_announce(bytes: &[u8]) -> Result<BlockAnnounceRef, DecodeBlockAnnounceError> {
let result: Result<_, nom::error::Error<_>> =
nom::combinator::all_consuming(nom::combinator::map(
nom::combinator::all_consuming(nom::combinator::complete(nom::combinator::map(
nom::sequence::tuple((
nom::combinator::recognize(|enc_hdr| match header::decode_partial(enc_hdr) {
Ok((hdr, rest)) => Ok((rest, hdr)),
Expand All @@ -112,7 +112,7 @@ pub fn decode_block_announce(bytes: &[u8]) -> Result<BlockAnnounceRef, DecodeBlo
scale_encoded_header,
is_best,
},
))(bytes)
)))(bytes)
.finish();

match result {
Expand Down Expand Up @@ -150,7 +150,7 @@ pub fn decode_block_announces_handshake(
handshake: &[u8],
) -> Result<BlockAnnouncesHandshakeRef, BlockAnnouncesHandshakeDecodeError> {
let result: Result<_, nom::error::Error<_>> =
nom::combinator::all_consuming(nom::combinator::map(
nom::combinator::all_consuming(nom::combinator::complete(nom::combinator::map(
nom::sequence::tuple((
nom::branch::alt((
nom::combinator::map(nom::bytes::complete::tag(&[0b1]), |_| Role::Full),
Expand All @@ -167,7 +167,7 @@ pub fn decode_block_announces_handshake(
best_hash: TryFrom::try_from(best_hash).unwrap(),
genesis_hash: TryFrom::try_from(genesis_hash).unwrap(),
},
))(handshake)
)))(handshake)
.finish();

match result {
Expand Down
21 changes: 17 additions & 4 deletions src/network/protocol/block_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,13 +142,17 @@ pub fn decode_block_request(
request_bytes: &[u8],
) -> Result<BlocksRequestConfig, DecodeBlockRequestError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode::<((_,), Option<_>, Option<_>, (_,), Option<_>), _, _>((
nom::combinator::complete(protobuf::message_decode::<
((_,), Option<_>, Option<_>, (_,), Option<_>),
_,
_,
>((
protobuf::uint32_tag_decode(1),
protobuf::bytes_tag_decode(2),
protobuf::bytes_tag_decode(3),
protobuf::enum_tag_decode(5),
protobuf::uint32_tag_decode(6),
)),
))),
);

let ((fields,), hash, number, (direction,), max_blocks) =
Expand Down Expand Up @@ -265,15 +269,15 @@ pub fn decode_block_response(
response_bytes: &[u8],
) -> Result<Vec<BlockData>, DecodeBlockResponseError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((protobuf::message_tag_decode(
nom::combinator::complete(protobuf::message_decode((protobuf::message_tag_decode(
1,
protobuf::message_decode::<((_,), (_,), Vec<_>, Option<_>), _, _>((
protobuf::bytes_tag_decode(1),
protobuf::bytes_tag_decode(2),
protobuf::bytes_tag_decode(3),
protobuf::bytes_tag_decode(8),
)),
),)),
),))),
);

let blocks: Vec<_> = match nom::Finish::finish(parser(response_bytes)) {
Expand Down Expand Up @@ -400,3 +404,12 @@ fn decode_justifications<'a, E: nom::error::ParseError<&'a [u8]>>(
)
})(bytes)
}

#[cfg(test)]
mod tests {
#[test]
fn regression_2339() {
// Regression test for https://github.com/paritytech/smoldot/issues/2339.
let _ = super::decode_block_request(&[26, 10]);
}
}
4 changes: 2 additions & 2 deletions src/network/protocol/call_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,10 +69,10 @@ pub fn decode_call_proof_response(
response_bytes: &[u8],
) -> Result<Vec<Vec<u8>>, DecodeCallProofResponseError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((protobuf::message_tag_decode(
nom::combinator::complete(protobuf::message_decode((protobuf::message_tag_decode(
1,
protobuf::message_decode((protobuf::bytes_tag_decode(2),)),
),)),
),))),
);

let proof: &[u8] = match nom::Finish::finish(parser(response_bytes)) {
Expand Down
6 changes: 5 additions & 1 deletion src/network/protocol/grandpa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ pub struct PrevoteRef<'a> {
pub fn decode_grandpa_notification(
scale_encoded: &[u8],
) -> Result<GrandpaNotificationRef, DecodeGrandpaNotificationError> {
match nom::combinator::all_consuming(grandpa_notification)(scale_encoded).finish() {
match nom::combinator::all_consuming(nom::combinator::complete(grandpa_notification))(
scale_encoded,
)
.finish()
{
Ok((_, notif)) => Ok(notif),
Err(err) => Err(DecodeGrandpaNotificationError(err.code)),
}
Expand Down
4 changes: 2 additions & 2 deletions src/network/protocol/identify.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,14 +116,14 @@ pub fn decode_identify_response(
DecodeIdentifyResponseError,
> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((
nom::combinator::complete(protobuf::message_decode((
protobuf::string_tag_decode(5),
protobuf::string_tag_decode(6),
protobuf::bytes_tag_decode(1),
protobuf::bytes_tag_decode(2),
protobuf::bytes_tag_decode(4),
protobuf::string_tag_decode(3),
)),
))),
);

let (
Expand Down
4 changes: 2 additions & 2 deletions src/network/protocol/state_request.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ pub fn decode_state_response(
response_bytes: &[u8],
) -> Result<Vec<StateResponseEntry>, DecodeStateResponseError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((protobuf::message_tag_decode(
nom::combinator::complete(protobuf::message_decode((protobuf::message_tag_decode(
1,
protobuf::message_decode((
protobuf::bytes_tag_decode(1),
Expand All @@ -71,7 +71,7 @@ pub fn decode_state_response(
),
protobuf::bool_tag_decode(3),
)),
),)),
),))),
);

let entries: Vec<((&[u8],), Vec<((&[u8],), (&[u8],))>, (bool,))> =
Expand Down
4 changes: 2 additions & 2 deletions src/network/protocol/storage_proof.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ pub fn decode_storage_proof_response(
response_bytes: &[u8],
) -> Result<Vec<Vec<u8>>, DecodeStorageProofResponseError> {
let mut parser = nom::combinator::all_consuming::<_, _, nom::error::Error<&[u8]>, _>(
protobuf::message_decode((protobuf::message_tag_decode(
nom::combinator::complete(protobuf::message_decode((protobuf::message_tag_decode(
2,
protobuf::message_decode((protobuf::bytes_tag_decode(2),)),
),)),
),))),
);

let proof: &[u8] = match nom::Finish::finish(parser(response_bytes)) {
Expand Down