add checked instructions sysvar api (#20790)

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)?;
 

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)]

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);
+        let data = construct_instructions_data(message, demote_program_write_locks);
-        // add room for current instruction index.
-        data.resize(data.len() + 2, 0);
         let owner = if is_owned_by_sysvar {
             sysvar::id()
         } else {

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());
     });
 }

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<u8> {
+    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<Instruction, SanitizeError> {
     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<Instruction, ProgramError> {
+    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)
+        );
+    }
 }