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

Validate code when scheduling upgrades from registrar #3232

Merged
merged 9 commits into from
Feb 14, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
6 changes: 6 additions & 0 deletions polkadot/parachain/src/primitives.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 49 additions & 4 deletions polkadot/runtime/common/src/paras_registrar/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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(|| {
Expand Down Expand Up @@ -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);
Expand Down
6 changes: 6 additions & 0 deletions polkadot/runtime/parachains/src/paras/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I still think that this should be moved to schedule_code_upgrade.


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);
Expand Down
13 changes: 13 additions & 0 deletions prdoc/pr_3232.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: Validate code when scheduling uprades

doc:
- audience: Runtime User
description: |
Adds checks to ensure that the validation code is valid before scheduling
a code upgrade.

crates:
- name: polkadot-runtime-parachains
Loading