Ensure a bad datastream cannot cause problems (#701)

* Ensure a bad datastream cannot cause problems

* Formatting

* Formatting

Author: gavofyork

pallets/xcmp-queue/src/lib.rs

@@ -401,23 +401,26 @@ impl<T: Config> Pallet<T> {
 			XcmpMessageFormat::ConcatenatedEncodedBlob => {
 				while !remaining_fragments.is_empty() {
 					last_remaining_fragments = remaining_fragments;
-					if let Ok(blob) = <Vec<u8>>::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 <Vec<u8>>::decode_all(&mut remaining_fragments) {
+						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),
+								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""[..];
+						},
 					}
 				}
 			},

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, Test, XcmpQueue};
 
 #[test]
 fn one_message_does_not_panic() {
@@ -27,3 +27,35 @@ 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::<Test>::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);
+	});
+}
+
+#[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::<Test>::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);
+	});
+}