Fix #2339 (#2340)

* Fix #2339 * Add CHANGELOG * Add regression test Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
paritytech · Jun 7, 2022 · 57d4c65 · 57d4c65
1 parent b93cc09
commit 57d4c65
Show file tree

Hide file tree

Showing 11 changed files with 46 additions and 24 deletions.
diff --git a/bin/wasm-node/CHANGELOG.md b/bin/wasm-node/CHANGELOG.md
@@ -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

diff --git a/src/libp2p/connection/noise.rs b/src/libp2p/connection/noise.rs
@@ -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),

diff --git a/src/libp2p/peer_id.rs b/src/libp2p/peer_id.rs
@@ -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)),

diff --git a/src/network/kademlia.rs b/src/network/kademlia.rs
@@ -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,
@@ -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)) {

diff --git a/src/network/protocol/block_announces.rs b/src/network/protocol/block_announces.rs
@@ -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)),
@@ -112,7 +112,7 @@ pub fn decode_block_announce(bytes: &[u8]) -> Result<BlockAnnounceRef, DecodeBlo
                 scale_encoded_header,
                 is_best,
             },
-        ))(bytes)
+        )))(bytes)
         .finish();
 
     match result {
@@ -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),
@@ -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 {

diff --git a/src/network/protocol/block_request.rs b/src/network/protocol/block_request.rs
@@ -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) =
@@ -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)) {
@@ -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]);
+    }
+}
diff --git a/src/network/protocol/call_proof.rs b/src/network/protocol/call_proof.rs
@@ -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)) {

diff --git a/src/network/protocol/grandpa.rs b/src/network/protocol/grandpa.rs
@@ -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)),
     }

diff --git a/src/network/protocol/identify.rs b/src/network/protocol/identify.rs
@@ -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 (

diff --git a/src/network/protocol/state_request.rs b/src/network/protocol/state_request.rs
@@ -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),
@@ -71,7 +71,7 @@ pub fn decode_state_response(
                 ),
                 protobuf::bool_tag_decode(3),
             )),
-        ),)),
+        ),))),
     );
 
     let entries: Vec<((&[u8],), Vec<((&[u8],), (&[u8],))>, (bool,))> =

diff --git a/src/network/protocol/storage_proof.rs b/src/network/protocol/storage_proof.rs
@@ -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)) {