diff --git a/programs/bpf-loader-tests/tests/extend_program_data_ix.rs b/programs/bpf-loader-tests/tests/extend_program_data_ix.rs deleted file mode 100644 index d5c79c577e..0000000000 --- a/programs/bpf-loader-tests/tests/extend_program_data_ix.rs +++ /dev/null @@ -1,422 +0,0 @@ -use { - assert_matches::assert_matches, - common::{add_upgradeable_loader_account, assert_ix_error, setup_test_context}, - solana_program_test::*, - solana_sdk::{ - account::{AccountSharedData, ReadableAccount}, - account_utils::StateMut, - bpf_loader_upgradeable::{extend_program_data, id, UpgradeableLoaderState}, - instruction::InstructionError, - pubkey::Pubkey, - signature::{Keypair, Signer}, - system_instruction::{self, SystemError, MAX_PERMITTED_DATA_LENGTH}, - system_program, - transaction::Transaction, - }, -}; - -mod common; - -#[tokio::test] -async fn test_extend_program_data() { - let mut context = setup_test_context().await; - - let program_data_address = Pubkey::new_unique(); - let program_data_len = 100; - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - program_data_len, - ) - .await; - - let client = &mut context.banks_client; - let payer = &context.payer; - let recent_blockhash = context.last_blockhash; - const ADDITIONAL_BYTES: u32 = 42; - let transaction = Transaction::new_signed_with_payer( - &[extend_program_data( - &program_data_address, - Some(&payer.pubkey()), - ADDITIONAL_BYTES, - )], - Some(&payer.pubkey()), - &[payer], - recent_blockhash, - ); - - assert_matches!(client.process_transaction(transaction).await, Ok(())); - let updated_program_data_account = client - .get_account(program_data_address) - .await - .unwrap() - .unwrap(); - assert_eq!( - updated_program_data_account.data().len(), - program_data_len + ADDITIONAL_BYTES as usize - ); -} - -#[tokio::test] -async fn test_extend_program_data_not_upgradeable() { - let mut context = setup_test_context().await; - - let program_data_address = Pubkey::new_unique(); - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: None, - }, - 100, - ) - .await; - - let payer_address = context.payer.pubkey(); - assert_ix_error( - &mut context, - extend_program_data(&program_data_address, Some(&payer_address), 42), - None, - InstructionError::Immutable, - "should fail because the program data account isn't upgradeable", - ) - .await; -} - -#[tokio::test] -async fn test_extend_program_data_by_zero_bytes() { - let mut context = setup_test_context().await; - - let program_data_address = Pubkey::new_unique(); - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - 100, - ) - .await; - - let payer_address = context.payer.pubkey(); - assert_ix_error( - &mut context, - extend_program_data(&program_data_address, Some(&payer_address), 0), - None, - InstructionError::InvalidInstructionData, - "should fail because the program data account must be extended by more than 0 bytes", - ) - .await; -} - -#[tokio::test] -async fn test_extend_program_data_past_max_size() { - let mut context = setup_test_context().await; - - let program_data_address = Pubkey::new_unique(); - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - MAX_PERMITTED_DATA_LENGTH as usize, - ) - .await; - - let payer_address = context.payer.pubkey(); - assert_ix_error( - &mut context, - extend_program_data(&program_data_address, Some(&payer_address), 1), - None, - InstructionError::InvalidRealloc, - "should fail because the program data account cannot be extended past the max data size", - ) - .await; -} - -#[tokio::test] -async fn test_extend_program_data_with_invalid_payer() { - let mut context = setup_test_context().await; - let rent = context.banks_client.get_rent().await.unwrap(); - - let program_data_address = Pubkey::new_unique(); - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - 100, - ) - .await; - - let payer_with_sufficient_funds = Keypair::new(); - context.set_account( - &payer_with_sufficient_funds.pubkey(), - &AccountSharedData::new(10_000_000_000, 0, &system_program::id()), - ); - - let payer_with_insufficient_funds = Keypair::new(); - context.set_account( - &payer_with_insufficient_funds.pubkey(), - &AccountSharedData::new(rent.minimum_balance(0), 0, &system_program::id()), - ); - - let payer_with_invalid_owner = Keypair::new(); - context.set_account( - &payer_with_invalid_owner.pubkey(), - &AccountSharedData::new(rent.minimum_balance(0), 0, &id()), - ); - - assert_ix_error( - &mut context, - extend_program_data( - &program_data_address, - Some(&payer_with_insufficient_funds.pubkey()), - 1024, - ), - Some(&payer_with_insufficient_funds), - InstructionError::from(SystemError::ResultWithNegativeLamports), - "should fail because the payer has insufficient funds to cover program data account rent", - ) - .await; - - assert_ix_error( - &mut context, - extend_program_data( - &program_data_address, - Some(&payer_with_invalid_owner.pubkey()), - 1, - ), - Some(&payer_with_invalid_owner), - InstructionError::ExternalAccountLamportSpend, - "should fail because the payer is not a system account", - ) - .await; - - let mut ix = extend_program_data( - &program_data_address, - Some(&payer_with_sufficient_funds.pubkey()), - 1, - ); - - // Demote payer account meta to non-signer so that transaction signing succeeds - { - let payer_meta = ix - .accounts - .iter_mut() - .find(|meta| meta.pubkey == payer_with_sufficient_funds.pubkey()) - .expect("expected to find payer account meta"); - payer_meta.is_signer = false; - } - - assert_ix_error( - &mut context, - ix, - None, - InstructionError::PrivilegeEscalation, - "should fail because the payer did not sign", - ) - .await; -} - -#[tokio::test] -async fn test_extend_program_data_without_payer() { - let mut context = setup_test_context().await; - let rent = context.banks_client.get_rent().await.unwrap(); - - let program_data_address = Pubkey::new_unique(); - let program_data_len = 100; - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - program_data_len, - ) - .await; - - assert_ix_error( - &mut context, - extend_program_data(&program_data_address, None, 1024), - None, - InstructionError::NotEnoughAccountKeys, - "should fail because program data has insufficient funds to cover rent", - ) - .await; - - let client = &mut context.banks_client; - let payer = &context.payer; - let recent_blockhash = context.last_blockhash; - - const ADDITIONAL_BYTES: u32 = 42; - let min_balance_increase_for_extend = rent - .minimum_balance(ADDITIONAL_BYTES as usize) - .saturating_sub(rent.minimum_balance(0)); - - let transaction = Transaction::new_signed_with_payer( - &[ - system_instruction::transfer( - &payer.pubkey(), - &program_data_address, - min_balance_increase_for_extend, - ), - extend_program_data(&program_data_address, None, ADDITIONAL_BYTES), - ], - Some(&payer.pubkey()), - &[payer], - recent_blockhash, - ); - - assert_matches!(client.process_transaction(transaction).await, Ok(())); - let updated_program_data_account = client - .get_account(program_data_address) - .await - .unwrap() - .unwrap(); - assert_eq!( - updated_program_data_account.data().len(), - program_data_len + ADDITIONAL_BYTES as usize - ); -} - -#[tokio::test] -async fn test_extend_program_data_with_invalid_system_program() { - let mut context = setup_test_context().await; - - let program_data_address = Pubkey::new_unique(); - let program_data_len = 100; - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - program_data_len, - ) - .await; - - let payer_address = context.payer.pubkey(); - let mut ix = extend_program_data(&program_data_address, Some(&payer_address), 1); - - // Change system program to an invalid key - { - let system_program_meta = ix - .accounts - .iter_mut() - .find(|meta| meta.pubkey == crate::system_program::ID) - .expect("expected to find system program account meta"); - system_program_meta.pubkey = Pubkey::new_unique(); - } - - assert_ix_error( - &mut context, - ix, - None, - InstructionError::MissingAccount, - "should fail because the system program is missing", - ) - .await; -} - -#[tokio::test] -async fn test_extend_program_data_with_invalid_program_data() { - let mut context = setup_test_context().await; - let rent = context.banks_client.get_rent().await.unwrap(); - let payer_address = context.payer.pubkey(); - - let program_data_address = Pubkey::new_unique(); - add_upgradeable_loader_account( - &mut context, - &program_data_address, - &UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(Pubkey::new_unique()), - }, - 100, - ) - .await; - - let program_data_address_with_invalid_state = Pubkey::new_unique(); - { - let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &id()); - account - .set_state(&UpgradeableLoaderState::Buffer { - authority_address: Some(payer_address), - }) - .expect("serialization failed"); - context.set_account(&program_data_address_with_invalid_state, &account); - } - - let program_data_address_with_invalid_owner = Pubkey::new_unique(); - { - let invalid_owner = Pubkey::new_unique(); - let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &invalid_owner); - account - .set_state(&UpgradeableLoaderState::ProgramData { - slot: 0, - upgrade_authority_address: Some(payer_address), - }) - .expect("serialization failed"); - context.set_account(&program_data_address_with_invalid_owner, &account); - } - - assert_ix_error( - &mut context, - extend_program_data( - &program_data_address_with_invalid_state, - Some(&payer_address), - 1024, - ), - None, - InstructionError::InvalidAccountData, - "should fail because the program data account state isn't valid", - ) - .await; - - assert_ix_error( - &mut context, - extend_program_data( - &program_data_address_with_invalid_owner, - Some(&payer_address), - 1024, - ), - None, - InstructionError::InvalidAccountOwner, - "should fail because the program data account owner isn't valid", - ) - .await; - - let mut ix = extend_program_data(&program_data_address, Some(&payer_address), 1); - - // Demote ProgramData account meta to read-only - { - let program_data_meta = ix - .accounts - .iter_mut() - .find(|meta| meta.pubkey == program_data_address) - .expect("expected to find program data account meta"); - program_data_meta.is_writable = false; - } - - assert_ix_error( - &mut context, - ix, - None, - InstructionError::InvalidArgument, - "should fail because the program data account is not writable", - ) - .await; -} diff --git a/programs/bpf-loader-tests/tests/extend_program_ix.rs b/programs/bpf-loader-tests/tests/extend_program_ix.rs new file mode 100644 index 0000000000..7c928446e5 --- /dev/null +++ b/programs/bpf-loader-tests/tests/extend_program_ix.rs @@ -0,0 +1,715 @@ +use { + assert_matches::assert_matches, + common::{add_upgradeable_loader_account, assert_ix_error, setup_test_context}, + solana_program_test::*, + solana_sdk::{ + account::{AccountSharedData, ReadableAccount}, + account_utils::StateMut, + bpf_loader_upgradeable::{extend_program, id, UpgradeableLoaderState}, + instruction::InstructionError, + pubkey::Pubkey, + signature::{Keypair, Signer}, + system_instruction::{self, SystemError, MAX_PERMITTED_DATA_LENGTH}, + system_program, + transaction::Transaction, + }, +}; + +mod common; + +#[tokio::test] +async fn test_extend_program() { + let mut context = setup_test_context().await; + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + let program_data_len = 100; + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + program_data_len, + ) + .await; + + let client = &mut context.banks_client; + let payer = &context.payer; + let recent_blockhash = context.last_blockhash; + const ADDITIONAL_BYTES: u32 = 42; + let transaction = Transaction::new_signed_with_payer( + &[extend_program( + &program_address, + Some(&payer.pubkey()), + ADDITIONAL_BYTES, + )], + Some(&payer.pubkey()), + &[payer], + recent_blockhash, + ); + + assert_matches!(client.process_transaction(transaction).await, Ok(())); + let updated_program_data_account = client + .get_account(programdata_address) + .await + .unwrap() + .unwrap(); + assert_eq!( + updated_program_data_account.data().len(), + program_data_len + ADDITIONAL_BYTES as usize + ); +} + +#[tokio::test] +async fn test_extend_program_not_upgradeable() { + let mut context = setup_test_context().await; + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: None, + }, + 100, + ) + .await; + + let payer_address = context.payer.pubkey(); + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 42), + None, + InstructionError::Immutable, + "should fail because the program data account isn't upgradeable", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_by_zero_bytes() { + let mut context = setup_test_context().await; + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + let payer_address = context.payer.pubkey(); + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 0), + None, + InstructionError::InvalidInstructionData, + "should fail because the program data account must be extended by more than 0 bytes", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_past_max_size() { + let mut context = setup_test_context().await; + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + MAX_PERMITTED_DATA_LENGTH as usize, + ) + .await; + + let payer_address = context.payer.pubkey(); + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 1), + None, + InstructionError::InvalidRealloc, + "should fail because the program data account cannot be extended past the max data size", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_invalid_payer() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + let payer_with_sufficient_funds = Keypair::new(); + context.set_account( + &payer_with_sufficient_funds.pubkey(), + &AccountSharedData::new(10_000_000_000, 0, &system_program::id()), + ); + + let payer_with_insufficient_funds = Keypair::new(); + context.set_account( + &payer_with_insufficient_funds.pubkey(), + &AccountSharedData::new(rent.minimum_balance(0), 0, &system_program::id()), + ); + + let payer_with_invalid_owner = Keypair::new(); + context.set_account( + &payer_with_invalid_owner.pubkey(), + &AccountSharedData::new(rent.minimum_balance(0), 0, &id()), + ); + + assert_ix_error( + &mut context, + extend_program( + &program_address, + Some(&payer_with_insufficient_funds.pubkey()), + 1024, + ), + Some(&payer_with_insufficient_funds), + InstructionError::from(SystemError::ResultWithNegativeLamports), + "should fail because the payer has insufficient funds to cover program data account rent", + ) + .await; + + assert_ix_error( + &mut context, + extend_program( + &program_address, + Some(&payer_with_invalid_owner.pubkey()), + 1, + ), + Some(&payer_with_invalid_owner), + InstructionError::ExternalAccountLamportSpend, + "should fail because the payer is not a system account", + ) + .await; + + let mut ix = extend_program( + &program_address, + Some(&payer_with_sufficient_funds.pubkey()), + 1, + ); + + // Demote payer account meta to non-signer so that transaction signing succeeds + { + let payer_meta = ix + .accounts + .iter_mut() + .find(|meta| meta.pubkey == payer_with_sufficient_funds.pubkey()) + .expect("expected to find payer account meta"); + payer_meta.is_signer = false; + } + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::PrivilegeEscalation, + "should fail because the payer did not sign", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_without_payer() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + let program_data_len = 100; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + program_data_len, + ) + .await; + + assert_ix_error( + &mut context, + extend_program(&program_address, None, 1024), + None, + InstructionError::NotEnoughAccountKeys, + "should fail because program data has insufficient funds to cover rent", + ) + .await; + + let client = &mut context.banks_client; + let payer = &context.payer; + let recent_blockhash = context.last_blockhash; + + const ADDITIONAL_BYTES: u32 = 42; + let min_balance_increase_for_extend = rent + .minimum_balance(ADDITIONAL_BYTES as usize) + .saturating_sub(rent.minimum_balance(0)); + + let transaction = Transaction::new_signed_with_payer( + &[ + system_instruction::transfer( + &payer.pubkey(), + &programdata_address, + min_balance_increase_for_extend, + ), + extend_program(&program_address, None, ADDITIONAL_BYTES), + ], + Some(&payer.pubkey()), + &[payer], + recent_blockhash, + ); + + assert_matches!(client.process_transaction(transaction).await, Ok(())); + let updated_program_data_account = client + .get_account(programdata_address) + .await + .unwrap() + .unwrap(); + assert_eq!( + updated_program_data_account.data().len(), + program_data_len + ADDITIONAL_BYTES as usize + ); +} + +#[tokio::test] +async fn test_extend_program_with_invalid_system_program() { + let mut context = setup_test_context().await; + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + let program_data_len = 100; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + program_data_len, + ) + .await; + + let payer_address = context.payer.pubkey(); + let mut ix = extend_program(&program_address, Some(&payer_address), 1); + + // Change system program to an invalid key + { + let system_program_meta = ix + .accounts + .iter_mut() + .find(|meta| meta.pubkey == crate::system_program::ID) + .expect("expected to find system program account meta"); + system_program_meta.pubkey = Pubkey::new_unique(); + } + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::MissingAccount, + "should fail because the system program is missing", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_mismatch_program_data() { + let mut context = setup_test_context().await; + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + + let mismatch_programdata_address = Pubkey::new_unique(); + add_upgradeable_loader_account( + &mut context, + &mismatch_programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + let mut ix = extend_program(&program_address, Some(&payer_address), 1); + + // Replace ProgramData account meta with invalid account + { + let program_data_meta = ix + .accounts + .iter_mut() + .find(|meta| meta.pubkey == programdata_address) + .expect("expected to find program data account meta"); + program_data_meta.pubkey = mismatch_programdata_address; + } + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::InvalidArgument, + "should fail because the program data account doesn't match the program", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_readonly_program_data() { + let mut context = setup_test_context().await; + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + let mut ix = extend_program(&program_address, Some(&payer_address), 1); + + // Demote ProgramData account meta to read-only + { + let program_data_meta = ix + .accounts + .iter_mut() + .find(|meta| meta.pubkey == programdata_address) + .expect("expected to find program data account meta"); + program_data_meta.is_writable = false; + } + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::InvalidArgument, + "should fail because the program data account is not writable", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_invalid_program_data_state() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + + { + let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &id()); + account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(payer_address), + }) + .expect("serialization failed"); + context.set_account(&programdata_address, &account); + } + + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 1024), + None, + InstructionError::InvalidAccountData, + "should fail because the program data account state isn't valid", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_invalid_program_data_owner() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + + { + let invalid_owner = Pubkey::new_unique(); + let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &invalid_owner); + account + .set_state(&UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(payer_address), + }) + .expect("serialization failed"); + context.set_account(&programdata_address, &account); + } + + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 1024), + None, + InstructionError::InvalidAccountOwner, + "should fail because the program data account owner isn't valid", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_readonly_program() { + let mut context = setup_test_context().await; + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + add_upgradeable_loader_account( + &mut context, + &program_address, + &UpgradeableLoaderState::Program { + programdata_address, + }, + UpgradeableLoaderState::size_of_program(), + ) + .await; + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + let mut ix = extend_program(&program_address, Some(&payer_address), 1); + + // Demote Program account meta to read-only + { + let program_meta = ix + .accounts + .iter_mut() + .find(|meta| meta.pubkey == program_address) + .expect("expected to find program account meta"); + program_meta.is_writable = false; + } + + assert_ix_error( + &mut context, + ix, + None, + InstructionError::InvalidArgument, + "should fail because the program account is not writable", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_invalid_program_owner() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + + { + let invalid_owner = Pubkey::new_unique(); + let program_len = UpgradeableLoaderState::size_of_program(); + let mut account = AccountSharedData::new( + rent.minimum_balance(program_len), + program_len, + &invalid_owner, + ); + account + .set_state(&UpgradeableLoaderState::Program { + programdata_address, + }) + .expect("serialization failed"); + context.set_account(&program_address, &account); + } + + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 1024), + None, + InstructionError::InvalidAccountOwner, + "should fail because the program account owner isn't valid", + ) + .await; +} + +#[tokio::test] +async fn test_extend_program_with_invalid_program_state() { + let mut context = setup_test_context().await; + let rent = context.banks_client.get_rent().await.unwrap(); + let payer_address = context.payer.pubkey(); + + let program_address = Pubkey::new_unique(); + let (programdata_address, _) = Pubkey::find_program_address(&[program_address.as_ref()], &id()); + + { + let mut account = AccountSharedData::new(rent.minimum_balance(100), 100, &id()); + account + .set_state(&UpgradeableLoaderState::Buffer { + authority_address: Some(payer_address), + }) + .expect("serialization failed"); + context.set_account(&program_address, &account); + } + + add_upgradeable_loader_account( + &mut context, + &programdata_address, + &UpgradeableLoaderState::ProgramData { + slot: 0, + upgrade_authority_address: Some(Pubkey::new_unique()), + }, + 100, + ) + .await; + + assert_ix_error( + &mut context, + extend_program(&program_address, Some(&payer_address), 1024), + None, + InstructionError::InvalidAccountData, + "should fail because the program account state isn't valid", + ) + .await; +} diff --git a/programs/bpf_loader/src/lib.rs b/programs/bpf_loader/src/lib.rs index d37e11791e..ab6cf86de8 100644 --- a/programs/bpf_loader/src/lib.rs +++ b/programs/bpf_loader/src/lib.rs @@ -43,8 +43,8 @@ use { feature_set::{ cap_accounts_data_allocations_per_transaction, cap_bpf_program_instruction_accounts, disable_deploy_of_alloc_free_syscall, disable_deprecated_loader, - enable_bpf_loader_extend_program_data_ix, - error_on_syscall_bpf_function_hash_collisions, reject_callx_r10, + enable_bpf_loader_extend_program_ix, error_on_syscall_bpf_function_hash_collisions, + reject_callx_r10, }, instruction::{AccountMeta, InstructionError}, loader_instruction::LoaderInstruction, @@ -1032,10 +1032,10 @@ fn process_loader_upgradeable_instruction( } } } - UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes } => { + UpgradeableLoaderInstruction::ExtendProgram { additional_bytes } => { if !invoke_context .feature_set - .is_active(&enable_bpf_loader_extend_program_data_ix::ID) + .is_active(&enable_bpf_loader_extend_program_ix::ID) { return Err(InstructionError::InvalidInstructionData); } @@ -1046,10 +1046,11 @@ fn process_loader_upgradeable_instruction( } const PROGRAM_DATA_ACCOUNT_INDEX: IndexOfAccount = 0; + const PROGRAM_ACCOUNT_INDEX: IndexOfAccount = 1; #[allow(dead_code)] // System program is only required when a CPI is performed - const OPTIONAL_SYSTEM_PROGRAM_ACCOUNT_INDEX: IndexOfAccount = 1; - const OPTIONAL_PAYER_ACCOUNT_INDEX: IndexOfAccount = 2; + const OPTIONAL_SYSTEM_PROGRAM_ACCOUNT_INDEX: IndexOfAccount = 2; + const OPTIONAL_PAYER_ACCOUNT_INDEX: IndexOfAccount = 3; let programdata_account = instruction_context .try_borrow_instruction_account(transaction_context, PROGRAM_DATA_ACCOUNT_INDEX)?; @@ -1064,6 +1065,35 @@ fn process_loader_upgradeable_instruction( return Err(InstructionError::InvalidArgument); } + let program_account = instruction_context + .try_borrow_instruction_account(transaction_context, PROGRAM_ACCOUNT_INDEX)?; + if !program_account.is_writable() { + ic_logger_msg!(log_collector, "Program account is not writable"); + return Err(InstructionError::InvalidArgument); + } + if program_account.get_owner() != program_id { + ic_logger_msg!(log_collector, "Program account not owned by loader"); + return Err(InstructionError::InvalidAccountOwner); + } + match program_account.get_state()? { + UpgradeableLoaderState::Program { + programdata_address, + } => { + if programdata_address != programdata_key { + ic_logger_msg!( + log_collector, + "Program account does not match ProgramData account" + ); + return Err(InstructionError::InvalidArgument); + } + } + _ => { + ic_logger_msg!(log_collector, "Invalid Program account"); + return Err(InstructionError::InvalidAccountData); + } + } + drop(program_account); + let old_len = programdata_account.get_data().len(); let new_len = old_len.saturating_add(additional_bytes as usize); if new_len > MAX_PERMITTED_DATA_LENGTH as usize { diff --git a/sdk/program/src/bpf_loader_upgradeable.rs b/sdk/program/src/bpf_loader_upgradeable.rs index 19264adb05..a2d2aa4911 100644 --- a/sdk/program/src/bpf_loader_upgradeable.rs +++ b/sdk/program/src/bpf_loader_upgradeable.rs @@ -300,13 +300,19 @@ pub fn close_any( Instruction::new_with_bincode(id(), &UpgradeableLoaderInstruction::Close, metas) } -/// Returns the instruction required to extend the size of a program data account -pub fn extend_program_data( - program_data_address: &Pubkey, +/// Returns the instruction required to extend the size of a program's +/// executable data account +pub fn extend_program( + program_address: &Pubkey, payer_address: Option<&Pubkey>, additional_bytes: u32, ) -> Instruction { - let mut metas = vec![AccountMeta::new(*program_data_address, false)]; + let (program_data_address, _) = + Pubkey::find_program_address(&[program_address.as_ref()], &id()); + let mut metas = vec![ + AccountMeta::new(program_data_address, false), + AccountMeta::new(*program_address, false), + ]; if let Some(payer_address) = payer_address { metas.push(AccountMeta::new_readonly( crate::system_program::id(), @@ -316,7 +322,7 @@ pub fn extend_program_data( } Instruction::new_with_bincode( id(), - &UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes }, + &UpgradeableLoaderInstruction::ExtendProgram { additional_bytes }, metas, ) } diff --git a/sdk/program/src/loader_upgradeable_instruction.rs b/sdk/program/src/loader_upgradeable_instruction.rs index e751c40a13..b8832c5329 100644 --- a/sdk/program/src/loader_upgradeable_instruction.rs +++ b/sdk/program/src/loader_upgradeable_instruction.rs @@ -128,7 +128,7 @@ pub enum UpgradeableLoaderInstruction { /// is a ProgramData account. Close, - /// Extend a ProgramData account by the specified number of bytes. + /// Extend a program's ProgramData account by the specified number of bytes. /// Only upgradeable program's can be extended. /// /// The payer account must contain sufficient lamports to fund the @@ -138,11 +138,12 @@ pub enum UpgradeableLoaderInstruction { /// /// # Account references /// 0. `[writable]` The ProgramData account. - /// 1. `[]` System program (`solana_sdk::system_program::id()`), optional, used to transfer + /// 1. `[writable]` The ProgramData account's associated Program account. + /// 2. `[]` System program (`solana_sdk::system_program::id()`), optional, used to transfer /// lamports from the payer to the ProgramData account. - /// 2. `[signer]` The payer account, optional, that will pay necessary rent exemption costs + /// 3. `[signer]` The payer account, optional, that will pay necessary rent exemption costs /// for the increased storage size. - ExtendProgramData { + ExtendProgram { /// Number of bytes to extend the program data. additional_bytes: u32, }, diff --git a/sdk/src/feature_set.rs b/sdk/src/feature_set.rs index f2f3be0332..18857f4512 100644 --- a/sdk/src/feature_set.rs +++ b/sdk/src/feature_set.rs @@ -454,7 +454,7 @@ pub mod preserve_rent_epoch_for_rent_exempt_accounts { solana_sdk::declare_id!("HH3MUYReL2BvqqA3oEcAa7txju5GY6G4nxJ51zvsEjEZ"); } -pub mod enable_bpf_loader_extend_program_data_ix { +pub mod enable_bpf_loader_extend_program_ix { solana_sdk::declare_id!("8Zs9W7D9MpSEtUWSQdGniZk2cNmV22y6FLJwCx53asme"); } @@ -638,7 +638,7 @@ lazy_static! { (cap_accounts_data_size_per_block::id(), "cap the accounts data size per block #25517"), (stake_redelegate_instruction::id(), "enable the redelegate stake instruction #26294"), (preserve_rent_epoch_for_rent_exempt_accounts::id(), "preserve rent epoch for rent exempt accounts #26479"), - (enable_bpf_loader_extend_program_data_ix::id(), "enable bpf upgradeable loader ExtendProgramData instruction #25234"), + (enable_bpf_loader_extend_program_ix::id(), "enable bpf upgradeable loader ExtendProgram instruction #25234"), (enable_early_verification_of_account_modifications::id(), "enable early verification of account modifications #25899"), (prevent_crediting_accounts_that_end_rent_paying::id(), "prevent crediting rent paying accounts #26606"), (cap_bpf_program_instruction_accounts::id(), "enforce max number of accounts per bpf program instruction #26628"), diff --git a/transaction-status/src/parse_bpf_loader.rs b/transaction-status/src/parse_bpf_loader.rs index 69c302cc29..ee1a744599 100644 --- a/transaction-status/src/parse_bpf_loader.rs +++ b/transaction-status/src/parse_bpf_loader.rs @@ -155,19 +155,24 @@ pub fn parse_bpf_upgradeable_loader( }), }) } - UpgradeableLoaderInstruction::ExtendProgramData { additional_bytes } => { + UpgradeableLoaderInstruction::ExtendProgram { additional_bytes } => { check_num_bpf_upgradeable_loader_accounts(&instruction.accounts, 2)?; Ok(ParsedInstructionEnum { - instruction_type: "extendProgramData".to_string(), + instruction_type: "extendProgram".to_string(), info: json!({ "additionalBytes": additional_bytes, "programDataAccount": account_keys[instruction.accounts[0] as usize].to_string(), - "systemProgram": account_keys[instruction.accounts[1] as usize].to_string(), - "payerAccount": if instruction.accounts.len() > 2 { + "programAccount": account_keys[instruction.accounts[1] as usize].to_string(), + "systemProgram": if instruction.accounts.len() > 3 { Some(account_keys[instruction.accounts[2] as usize].to_string()) } else { None }, + "payerAccount": if instruction.accounts.len() > 4 { + Some(account_keys[instruction.accounts[3] as usize].to_string()) + } else { + None + }, }), }) }