diff --git a/programs/bpf/rust/instruction_introspection/src/lib.rs b/programs/bpf/rust/instruction_introspection/src/lib.rs index 628c08b2d..3798379a6 100644 --- a/programs/bpf/rust/instruction_introspection/src/lib.rs +++ b/programs/bpf/rust/instruction_introspection/src/lib.rs @@ -33,9 +33,9 @@ fn process_instruction( return Err(ProgramError::InvalidAccountData); } - let instruction = instructions::load_instruction_at( + let instruction = instructions::load_instruction_at_checked( secp_instruction_index as usize, - &instruction_accounts.try_borrow_data()?, + instruction_accounts, ) .map_err(|_| ProgramError::InvalidAccountData)?; diff --git a/programs/bpf/rust/sysvar/src/lib.rs b/programs/bpf/rust/sysvar/src/lib.rs index a7ef6da22..6881edc82 100644 --- a/programs/bpf/rust/sysvar/src/lib.rs +++ b/programs/bpf/rust/sysvar/src/lib.rs @@ -7,6 +7,7 @@ use solana_program::{ account_info::AccountInfo, entrypoint, entrypoint::ProgramResult, + instruction::{AccountMeta, Instruction}, msg, program_error::ProgramError, pubkey::Pubkey, @@ -19,7 +20,7 @@ use solana_program::{ entrypoint!(process_instruction); #[allow(clippy::unnecessary_wraps)] pub fn process_instruction( - _program_id: &Pubkey, + program_id: &Pubkey, accounts: &[AccountInfo], _instruction_data: &[u8], ) -> ProgramResult { @@ -48,7 +49,27 @@ pub fn process_instruction( sysvar::instructions::id().log(); assert_eq!(*accounts[4].owner, sysvar::id()); let index = instructions::load_current_index(&accounts[4].try_borrow_data()?); + let instruction = instructions::load_instruction_at_checked(index as usize, &accounts[4])?; assert_eq!(0, index); + assert_eq!( + instruction, + Instruction::new_with_bytes( + *program_id, + &[] as &[u8], + vec![ + AccountMeta::new(*accounts[0].key, true), + AccountMeta::new(*accounts[1].key, false), + AccountMeta::new_readonly(*accounts[2].key, false), + AccountMeta::new_readonly(*accounts[3].key, false), + AccountMeta::new_readonly(*accounts[4].key, false), + AccountMeta::new_readonly(*accounts[5].key, false), + AccountMeta::new_readonly(*accounts[6].key, false), + AccountMeta::new_readonly(*accounts[7].key, false), + AccountMeta::new_readonly(*accounts[8].key, false), + AccountMeta::new_readonly(*accounts[9].key, false), + ], + ) + ); // Recent Blockhashes #[allow(deprecated)] diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index fd8256f54..b6ba7c726 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -35,6 +35,7 @@ use solana_sdk::{ nonce::NONCED_TX_MARKER_IX_INDEX, pubkey::Pubkey, system_program, sysvar, + sysvar::instructions::construct_instructions_data, transaction::{Result, SanitizedTransaction, TransactionError}, }; use std::{ @@ -206,9 +207,7 @@ impl Accounts { is_owned_by_sysvar: bool, demote_program_write_locks: bool, ) -> AccountSharedData { - let mut data = message.serialize_instructions(demote_program_write_locks); - // add room for current instruction index. - data.resize(data.len() + 2, 0); + let data = construct_instructions_data(message, demote_program_write_locks); let owner = if is_owned_by_sysvar { sysvar::id() } else { diff --git a/sdk/benches/serialize_instructions.rs b/sdk/benches/serialize_instructions.rs index 2d639bf01..7495a313c 100644 --- a/sdk/benches/serialize_instructions.rs +++ b/sdk/benches/serialize_instructions.rs @@ -54,6 +54,7 @@ fn bench_manual_instruction_deserialize(b: &mut Bencher) { let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS); b.iter(|| { for i in 0..instructions.len() { + #[allow(deprecated)] test::black_box(instructions::load_instruction_at(i, &serialized).unwrap()); } }); @@ -67,6 +68,7 @@ fn bench_manual_instruction_deserialize_single(b: &mut Bencher) { .unwrap(); let serialized = message.serialize_instructions(DEMOTE_PROGRAM_WRITE_LOCKS); b.iter(|| { + #[allow(deprecated)] test::black_box(instructions::load_instruction_at(3, &serialized).unwrap()); }); } diff --git a/sdk/program/src/sysvar/instructions.rs b/sdk/program/src/sysvar/instructions.rs index c211299df..ff4ab0cce 100644 --- a/sdk/program/src/sysvar/instructions.rs +++ b/sdk/program/src/sysvar/instructions.rs @@ -1,13 +1,29 @@ #![allow(clippy::integer_arithmetic)] //! This account contains the serialized transaction instructions -use crate::{instruction::Instruction, sanitize::SanitizeError}; +use crate::{ + account_info::AccountInfo, instruction::Instruction, program_error::ProgramError, + sanitize::SanitizeError, +}; // Instructions Sysvar, dummy type, use the associated helpers instead of the Sysvar trait pub struct Instructions(); crate::declare_sysvar_id!("Sysvar1nstructions1111111111111111111111111", Instructions); +// Construct the account data for the Instruction sSysvar +#[cfg(not(target_arch = "bpf"))] +pub fn construct_instructions_data( + message: &crate::message::SanitizedMessage, + demote_program_write_locks: bool, +) -> Vec { + let mut data = message.serialize_instructions(demote_program_write_locks); + // add room for current instruction index. + data.resize(data.len() + 2, 0); + + data +} + /// Load the current instruction's index from the Instructions Sysvar data pub fn load_current_index(data: &[u8]) -> u16 { let mut instr_fixed_data = [0u8; 2]; @@ -23,13 +39,37 @@ pub fn store_current_index(data: &mut [u8], instruction_index: u16) { } /// Load an instruction at the specified index +#[deprecated( + since = "1.8.0", + note = "Unsafe because the sysvar accounts address is not checked, please use `load_instruction_at_checked` instead" +)] pub fn load_instruction_at(index: usize, data: &[u8]) -> Result { crate::message::Message::deserialize_instruction(index, data) } +/// Load an instruction at the specified index +pub fn load_instruction_at_checked( + index: usize, + instruction_sysvar_account_info: &AccountInfo, +) -> Result { + if !check_id(instruction_sysvar_account_info.key) { + return Err(ProgramError::UnsupportedSysvar); + } + + let instruction_sysvar = instruction_sysvar_account_info.try_borrow_data()?; + crate::message::Message::deserialize_instruction(index, &instruction_sysvar).map_err(|err| { + match err { + SanitizeError::IndexOutOfBounds => ProgramError::InvalidArgument, + _ => ProgramError::InvalidInstructionData, + } + }) +} + #[cfg(test)] mod tests { use super::*; + use crate::{instruction::AccountMeta, message::Message, pubkey::Pubkey}; + use std::convert::TryFrom; #[test] fn test_load_store_instruction() { @@ -38,4 +78,51 @@ mod tests { assert_eq!(load_current_index(&data), 3); assert_eq!([4u8; 8], data[0..8]); } + + #[test] + fn test_load_instruction_at_checked() { + let instruction1 = Instruction::new_with_bincode( + Pubkey::new_unique(), + &0, + vec![AccountMeta::new(Pubkey::new_unique(), false)], + ); + let instruction2 = Instruction::new_with_bincode( + Pubkey::new_unique(), + &0, + vec![AccountMeta::new(Pubkey::new_unique(), false)], + ); + let sanitized_message = crate::message::SanitizedMessage::try_from(Message::new( + &[instruction1.clone(), instruction2.clone()], + Some(&Pubkey::new_unique()), + )) + .unwrap(); + + let key = id(); + let mut lamports = 0; + let mut data = construct_instructions_data(&sanitized_message, true); + let owner = crate::sysvar::id(); + let account_info = AccountInfo::new( + &key, + false, + false, + &mut lamports, + &mut data, + &owner, + false, + 0, + ); + + assert_eq!( + instruction1, + load_instruction_at_checked(0, &account_info).unwrap() + ); + assert_eq!( + instruction2, + load_instruction_at_checked(1, &account_info).unwrap() + ); + assert_eq!( + Err(ProgramError::InvalidArgument), + load_instruction_at_checked(2, &account_info) + ); + } }