From 5a890052fe2829181a0b704d80b64449374f1adf Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Oct 2021 13:44:06 +0200 Subject: [PATCH 1/3] Ensure a bad datastream cannot cause problems --- pallets/xcmp-queue/src/lib.rs | 37 ++++++++++++++++++--------------- pallets/xcmp-queue/src/tests.rs | 18 +++++++++++++++- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 721a2af1dbd..0b00b21bf35 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -401,23 +401,26 @@ impl Pallet { XcmpMessageFormat::ConcatenatedEncodedBlob => { while !remaining_fragments.is_empty() { last_remaining_fragments = remaining_fragments; - if let Ok(blob) = >::decode_all(&mut remaining_fragments) { - let weight = max_weight - weight_used; - match Self::handle_blob_message(sender, sent_at, blob, weight) { - Ok(used) => weight_used = weight_used.saturating_add(used), - Err(true) => { - // That message didn't get processed this time because of being - // too heavy. We leave it around for next time and bail. - remaining_fragments = last_remaining_fragments; - break - }, - Err(false) => { - // Message invalid; don't attempt to retry - }, - } - } else { - debug_assert!(false, "Invalid incoming blob message data"); - remaining_fragments = &b""[..]; + match >::decode_all(&mut remaining_fragments) { + Ok(blob) if blob.len() > 0 => { + let weight = max_weight - weight_used; + match Self::handle_blob_message(sender, sent_at, blob, weight) { + Ok(used) => weight_used = weight_used.saturating_add(used), + Err(true) => { + // That message didn't get processed this time because of being + // too heavy. We leave it around for next time and bail. + remaining_fragments = last_remaining_fragments; + break + }, + Err(false) => { + // Message invalid; don't attempt to retry + }, + } + }, + _ => { + debug_assert!(false, "Invalid incoming blob message data"); + remaining_fragments = &b""[..]; + }, } } }, diff --git a/pallets/xcmp-queue/src/tests.rs b/pallets/xcmp-queue/src/tests.rs index 41c2be10e8e..48e180e4093 100644 --- a/pallets/xcmp-queue/src/tests.rs +++ b/pallets/xcmp-queue/src/tests.rs @@ -15,7 +15,7 @@ use super::*; use cumulus_primitives_core::XcmpMessageHandler; -use mock::{new_test_ext, XcmpQueue}; +use mock::{new_test_ext, XcmpQueue, Test}; #[test] fn one_message_does_not_panic() { @@ -27,3 +27,19 @@ fn one_message_does_not_panic() { XcmpQueue::handle_xcmp_messages(messages.into_iter(), Weight::max_value()); }) } + +#[test] +#[should_panic = "Invalid incoming blob message data"] +fn bad_message_is_handled() { + new_test_ext().execute_with(|| { + let bad_data = vec![ + 1, 1, 3, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 64, 239, 139, 0, 0, + 0, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 37, 0, 0, + 0, 0, 0, 0, 0, 16, 0, 127, 147 + ]; + InboundXcmpMessages::::insert(ParaId::from(1000), 1, bad_data); + let format = XcmpMessageFormat::ConcatenatedEncodedBlob; + // This should exit with an error. + XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); + }); +} \ No newline at end of file From 37106ef7b6bd2583aa8dccc03a96fbfabe3a5730 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Oct 2021 16:33:41 +0200 Subject: [PATCH 2/3] Formatting --- pallets/xcmp-queue/src/tests.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pallets/xcmp-queue/src/tests.rs b/pallets/xcmp-queue/src/tests.rs index 48e180e4093..0d81e633660 100644 --- a/pallets/xcmp-queue/src/tests.rs +++ b/pallets/xcmp-queue/src/tests.rs @@ -15,7 +15,7 @@ use super::*; use cumulus_primitives_core::XcmpMessageHandler; -use mock::{new_test_ext, XcmpQueue, Test}; +use mock::{new_test_ext, Test, XcmpQueue}; #[test] fn one_message_does_not_panic() { @@ -33,13 +33,13 @@ fn one_message_does_not_panic() { fn bad_message_is_handled() { new_test_ext().execute_with(|| { let bad_data = vec![ - 1, 1, 3, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 64, 239, 139, 0, 0, - 0, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 37, 0, 0, - 0, 0, 0, 0, 0, 16, 0, 127, 147 + 1, 1, 3, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 64, 239, 139, 0, + 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, 37, 0, + 0, 0, 0, 0, 0, 0, 16, 0, 127, 147, ]; InboundXcmpMessages::::insert(ParaId::from(1000), 1, bad_data); let format = XcmpMessageFormat::ConcatenatedEncodedBlob; // This should exit with an error. XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); }); -} \ No newline at end of file +} From 28199bdd6d806072ddb01c77f5aa7c0808228d58 Mon Sep 17 00:00:00 2001 From: Gav Wood Date: Tue, 26 Oct 2021 21:15:08 +0200 Subject: [PATCH 3/3] Formatting --- pallets/xcmp-queue/src/lib.rs | 2 +- pallets/xcmp-queue/src/tests.rs | 16 ++++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/pallets/xcmp-queue/src/lib.rs b/pallets/xcmp-queue/src/lib.rs index 0b00b21bf35..378bfe383e0 100644 --- a/pallets/xcmp-queue/src/lib.rs +++ b/pallets/xcmp-queue/src/lib.rs @@ -402,7 +402,7 @@ impl Pallet { while !remaining_fragments.is_empty() { last_remaining_fragments = remaining_fragments; match >::decode_all(&mut remaining_fragments) { - Ok(blob) if blob.len() > 0 => { + Ok(blob) if remaining_fragments.len() < last_remaining_fragments.len() => { let weight = max_weight - weight_used; match Self::handle_blob_message(sender, sent_at, blob, weight) { Ok(used) => weight_used = weight_used.saturating_add(used), diff --git a/pallets/xcmp-queue/src/tests.rs b/pallets/xcmp-queue/src/tests.rs index 0d81e633660..9cc427848a6 100644 --- a/pallets/xcmp-queue/src/tests.rs +++ b/pallets/xcmp-queue/src/tests.rs @@ -43,3 +43,19 @@ fn bad_message_is_handled() { XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); }); } + +#[test] +#[should_panic = "Invalid incoming blob message data"] +fn other_bad_message_is_handled() { + new_test_ext().execute_with(|| { + let bad_data = vec![ + 1, 1, 1, 1, 3, 0, 0, 0, 0, 0, 0, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 64, 239, + 139, 0, 0, 0, 64, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 255, 255, 0, 0, 0, 0, 0, 0, 0, + 37, 0, 0, 0, 0, 0, 0, 0, 16, 0, 127, 147, + ]; + InboundXcmpMessages::::insert(ParaId::from(1000), 1, bad_data); + let format = XcmpMessageFormat::ConcatenatedEncodedBlob; + // This should exit with an error. + XcmpQueue::process_xcmp_message(1000.into(), (1, format), 10_000_000_000); + }); +}