Validate code when scheduling upgrades from registrar

polkadot/parachain/src/primitives.rs

@@ -78,6 +78,12 @@ impl ValidationCode {
 	pub fn hash(&self) -> ValidationCodeHash {
 		ValidationCodeHash(sp_runtime::traits::BlakeTwo256::hash(&self.0[..]))
 	}
+
+	/// Performs basic sanity checks on the validation code.
+	pub fn check_sanity(&self) -> bool {
+		// Compressed or not the wasm blob can never be less than 9 bytes.
+		self.0.len() >= 9
+	}
 }
 
 /// Unit type wrapper around [`type@Hash`] that represents the blake2-256 hash

polkadot/primitives/test-helpers/src/lib.rs

@@ -126,7 +126,7 @@ pub fn dummy_candidate_descriptor<H: AsRef<[u8]>>(relay_parent: H) -> CandidateD
 
 /// Create meaningless validation code.
 pub fn dummy_validation_code() -> ValidationCode {
-	ValidationCode(vec![1, 2, 3])
+	ValidationCode(vec![1, 2, 3, 4, 5, 6, 7, 8, 9])
 }
 
 /// Create meaningless head data.

polkadot/runtime/common/src/paras_registrar/mod.rs

@@ -182,8 +182,8 @@ pub mod pallet {
 		ParaLocked,
 		/// The ID given for registration has not been reserved.
 		NotReserved,
-		/// Registering parachain with empty code is not allowed.
-		EmptyCode,
+		/// The validation code is invalid.
+		InvalidCode,
 		/// Cannot perform a parachain slot / lifecycle swap. Check that the state of both paras
 		/// are correct for the swap to work.
 		CannotSwap,
@@ -657,7 +657,7 @@ impl<T: Config> Pallet<T> {
 		para_kind: ParaKind,
 	) -> Result<(ParaGenesisArgs, BalanceOf<T>), sp_runtime::DispatchError> {
 		let config = configuration::Pallet::<T>::config();
-		ensure!(validation_code.0.len() > 0, Error::<T>::EmptyCode);
+		ensure!(validation_code.0.len() > 0, Error::<T>::InvalidCode);
 		ensure!(validation_code.0.len() <= config.max_code_size as usize, Error::<T>::CodeTooLarge);
 		ensure!(
 			genesis_head.0.len() <= config.max_head_data_size as usize,
@@ -1032,6 +1032,51 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn schedule_code_upgrade_validates_code() {
+		new_test_ext().execute_with(|| {
+			const START_SESSION_INDEX: SessionIndex = 1;
+			run_to_session(START_SESSION_INDEX);
+
+			let para_id = LOWEST_PUBLIC_ID;
+			assert!(!Parachains::is_parathread(para_id));
+
+			let validation_code = test_validation_code(32);
+			assert_ok!(Registrar::reserve(RuntimeOrigin::signed(1)));
+			assert_eq!(Balances::reserved_balance(&1), <Test as Config>::ParaDeposit::get());
+			assert_ok!(Registrar::register(
+				RuntimeOrigin::signed(1),
+				para_id,
+				test_genesis_head(32),
+				validation_code.clone(),
+			));
+			conclude_pvf_checking::<Test>(&validation_code, VALIDATORS, START_SESSION_INDEX);
+
+			run_to_session(START_SESSION_INDEX + 2);
+			assert!(Parachains::is_parathread(para_id));
+
+			let new_code = test_validation_code(0);
+			assert_noop!(
+				Registrar::schedule_code_upgrade(
+					RuntimeOrigin::signed(1),
+					para_id,
+					new_code.clone(),
+				),
+				paras::Error::<Test>::InvalidCode
+			);
+
+			let new_code = test_validation_code(max_code_size() as usize + 1);
+			assert_noop!(
+				Registrar::schedule_code_upgrade(
+					RuntimeOrigin::signed(1),
+					para_id,
+					new_code.clone(),
+				),
+				paras::Error::<Test>::InvalidCode
+			);
+		});
+	}
+
 	#[test]
 	fn register_handles_basic_errors() {
 		new_test_ext().execute_with(|| {
@@ -1310,7 +1355,7 @@ mod tests {
 				RuntimeOrigin::signed(1),
 				para_id,
 				vec![1; 3].into(),
-				vec![1, 2, 3].into(),
+				test_validation_code(32)
 			));
 
 			assert_noop!(Registrar::add_lock(RuntimeOrigin::signed(2), para_id), BadOrigin);

polkadot/runtime/parachains/src/configuration.rs

@@ -281,7 +281,7 @@ impl<BlockNumber: Default + From<u32>> Default for HostConfiguration<BlockNumber
 			validation_upgrade_cooldown: Default::default(),
 			validation_upgrade_delay: 2u32.into(),
 			code_retention_period: Default::default(),
-			max_code_size: Default::default(),
+			max_code_size: 9u32.into(),
 			max_pov_size: Default::default(),
 			max_head_data_size: Default::default(),
 			coretime_cores: Default::default(),

polkadot/runtime/parachains/src/inclusion/tests.rs

@@ -1233,7 +1233,7 @@ fn candidate_checks() {
 				para_id: chain_a,
 				relay_parent: System::parent_hash(),
 				pov_hash: Hash::repeat_byte(1),
-				new_validation_code: Some(vec![5, 6, 7, 8].into()),
+				new_validation_code: Some(dummy_validation_code()),
 				persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
 				hrmp_watermark: RELAY_PARENT_NUM,
 				..Default::default()
@@ -1257,7 +1257,7 @@ fn candidate_checks() {
 				assert_eq!(expected_at, 12);
 				Paras::schedule_code_upgrade(
 					chain_a,
-					vec![1, 2, 3, 4].into(),
+					vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into(),
 					expected_at,
 					&cfg,
 					SetGoAhead::Yes,
@@ -1317,7 +1317,7 @@ fn candidate_checks() {
 				pov_hash: Hash::repeat_byte(1),
 				persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
 				hrmp_watermark: RELAY_PARENT_NUM,
-				validation_code: ValidationCode(vec![1]),
+				validation_code: ValidationCode(vec![9, 8, 7, 6, 5, 4, 3, 2, 1]),
 				..Default::default()
 			}
 			.build();
@@ -1726,7 +1726,7 @@ fn can_include_candidate_with_ok_code_upgrade() {
 			relay_parent: System::parent_hash(),
 			pov_hash: Hash::repeat_byte(1),
 			persisted_validation_data_hash: make_vdata_hash(chain_a).unwrap(),
-			new_validation_code: Some(vec![1, 2, 3].into()),
+			new_validation_code: Some(vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into()),
 			hrmp_watermark: RELAY_PARENT_NUM,
 			..Default::default()
 		}
@@ -2133,7 +2133,7 @@ fn para_upgrade_delay_scheduled_from_inclusion() {
 		shared::Pallet::<Test>::set_active_validators_ascending(validator_public.clone());
 		shared::Pallet::<Test>::set_session_index(5);
 
-		let new_validation_code: ValidationCode = vec![1, 2, 3, 4, 5].into();
+		let new_validation_code: ValidationCode = vec![9, 8, 7, 6, 5, 4, 3, 2, 1].into();
 		let new_validation_code_hash = new_validation_code.hash();
 
 		// Otherwise upgrade is no-op.

polkadot/runtime/parachains/src/paras/mod.rs

@@ -679,6 +679,8 @@ pub mod pallet {
 		PvfCheckSubjectInvalid,
 		/// Parachain cannot currently schedule a code upgrade.
 		CannotUpgradeCode,
+		/// Invalid validation code size.
+		InvalidCode,
 	}
 
 	/// All currently active PVF pre-checking votes.
@@ -1230,6 +1232,10 @@ impl<T: Config> Pallet<T> {
 		// Check that we can schedule an upgrade at all.
 		ensure!(Self::can_upgrade_validation_code(id), Error::<T>::CannotUpgradeCode);
 		let config = configuration::Pallet::<T>::config();
+		// Validation code sanity checks:
+		ensure!(new_code.check_sanity(), Error::<T>::InvalidCode);
+		ensure!(new_code.0.len() <= config.max_code_size as usize, Error::<T>::InvalidCode);
+
 		let current_block = frame_system::Pallet::<T>::block_number();
 		// Schedule the upgrade with a delay just like if a parachain triggered the upgrade.
 		let upgrade_block = current_block.saturating_add(config.validation_upgrade_delay);
@@ -1890,7 +1896,13 @@ impl<T: Config> Pallet<T> {
 	) -> Weight {
 		let mut weight = T::DbWeight::get().reads(1);
 
-		// Enacting this should be prevented by the `can_schedule_upgrade`
+		// Should be prevented by checks in `schedule_code_upgrade_external`
+		if !new_code.check_sanity() || new_code.0.len() > cfg.max_code_size as usize {
+			log::warn!(target: LOG_TARGET, "attempted to schedule an upgrade with invalid new validation code",);
+			return weight
+		}
+
+		// Enacting this should be prevented by the `can_upgrade_validation_code`
 		if FutureCodeHash::<T>::contains_key(&id) {
 			// This branch should never be reached. Signalling an upgrade is disallowed for a para
 			// that already has one upgrade scheduled.