Ensure that account info address is not in an account (anza-xyz#3044)

programs/bpf_loader/src/syscalls/cpi.rs

@@ -321,12 +321,6 @@ impl<'a, 'b> CallerAccount<'a, 'b> {
             .saturating_sub(account_info as *const _ as *const u64 as u64);
 
         let ref_to_len_in_vm = if direct_mapping {
-            // In the same vein as the other check_account_info_pointer() checks, we don't lock this
-            // pointer to a specific address but we don't want it to be inside accounts, or callees
-            // might be able to write to the pointed memory.
-            if data_len_vm_addr >= ebpf::MM_INPUT_START {
-                return Err(SyscallError::InvalidPointer.into());
-            }
             VmValue::VmAddress {
                 vm_addr: data_len_vm_addr,
                 memory_mapping,
@@ -804,6 +798,21 @@ fn translate_account_infos<'a, T, F>(
 where
     F: Fn(&T) -> u64,
 {
+    let direct_mapping = invoke_context
+        .get_feature_set()
+        .is_active(&feature_set::bpf_account_data_direct_mapping::id());
+
+    // In the same vein as the other check_account_info_pointer() checks, we don't lock
+    // this pointer to a specific address but we don't want it to be inside accounts, or
+    // callees might be able to write to the pointed memory.
+    if direct_mapping
+        && account_infos_addr
+            .saturating_add(account_infos_len.saturating_mul(std::mem::size_of::<T>() as u64))
+            >= ebpf::MM_INPUT_START
+    {
+        return Err(SyscallError::InvalidPointer.into());
+    }
+
     let account_infos = translate_slice::<T>(
         memory_mapping,
         account_infos_addr,

programs/sbf/c/src/invoke/invoke.c

@@ -38,6 +38,8 @@ static const uint8_t TEST_CPI_INVALID_KEY_POINTER = 35;
 static const uint8_t TEST_CPI_INVALID_OWNER_POINTER = 36;
 static const uint8_t TEST_CPI_INVALID_LAMPORTS_POINTER = 37;
 static const uint8_t TEST_CPI_INVALID_DATA_POINTER = 38;
+static const uint8_t TEST_WRITE_ACCOUNT = 40;
+static const uint8_t TEST_ACCOUNT_INFO_IN_ACCOUNT = 43;
 
 static const int MINT_INDEX = 0;
 static const int ARGUMENT_INDEX = 1;
@@ -748,6 +750,39 @@ extern uint64_t entrypoint(const uint8_t *input) {
     sol_invoke(&instruction, accounts, 4);
     break;
   }
+  case TEST_WRITE_ACCOUNT:
+  {
+    sol_log("Test TEST_WRITE_ACCOUNT");
+
+    uint8_t target_account_index = params.data[1];
+    uint64_t offset = *((uint64_t*)(params.data + 2));
+
+    accounts[target_account_index].data[offset] = params.data[10];
+    break;
+  }
+  case TEST_ACCOUNT_INFO_IN_ACCOUNT:
+  {
+    sol_log("Test TEST_ACCOUNT_INFO_IN_ACCOUNT");
+
+    uint8_t data[] = { TEST_WRITE_ACCOUNT, 1,  1, 0, 0, 0, 0, 0, 0, 0,  1 };
+
+    void *account_info_acc = accounts[1].data + 32;
+
+    sol_memcpy(account_info_acc, accounts, sizeof(accounts[0]) * params.ka_num);
+
+    SolAccountMeta arguments[] = {
+        {accounts[INVOKED_PROGRAM_INDEX].key, false, false},
+        {accounts[ARGUMENT_INDEX].key, true, false},
+        {accounts[MINT_INDEX].key, false, false},
+    };
+
+    const SolInstruction instruction = {accounts[INVOKED_PROGRAM_INDEX].key,
+                                        arguments, SOL_ARRAY_SIZE(arguments),
+                                        data, SOL_ARRAY_SIZE(data)};
+
+    sol_invoke(&instruction, account_info_acc, params.ka_num);
+    break;
+  }
 
   default:
     sol_panic();

programs/sbf/rust/invoke/src/lib.rs

@@ -1484,6 +1484,44 @@ fn process_instruction<'a>(
             )
             .unwrap();
         }
+        TEST_ACCOUNT_INFO_IN_ACCOUNT => {
+            msg!("TEST_ACCOUNT_INFO_IN_ACCOUNT");
+
+            let account_offset = usize::from_le_bytes(instruction_data[1..9].try_into().unwrap());
+
+            let mut instruction_data = vec![TEST_WRITE_ACCOUNT, 1];
+            instruction_data.extend_from_slice(&1u64.to_le_bytes());
+            instruction_data.push(1);
+
+            let data = accounts[ARGUMENT_INDEX].data.borrow().as_ptr();
+            let len = accounts.len();
+
+            let account_info_in_account: &mut [AccountInfo] = unsafe {
+                std::slice::from_raw_parts_mut(data.add(account_offset) as *mut AccountInfo, len)
+            };
+
+            unsafe {
+                std::ptr::copy_nonoverlapping(
+                    accounts.as_ptr(),
+                    account_info_in_account.as_mut_ptr(),
+                    len,
+                );
+            }
+
+            invoke(
+                &create_instruction(
+                    *program_id,
+                    &[
+                        (program_id, false, false),
+                        (accounts[1].key, true, false),
+                        (accounts[0].key, false, false),
+                    ],
+                    instruction_data.to_vec(),
+                ),
+                account_info_in_account,
+            )
+            .unwrap();
+        }
         _ => panic!("unexpected program data"),
     }
 

programs/sbf/rust/invoke_dep/src/lib.rs

@@ -42,6 +42,7 @@ pub const TEST_CPI_CHANGE_ACCOUNT_DATA_MEMORY_ALLOCATION: u8 = 39;
 pub const TEST_WRITE_ACCOUNT: u8 = 40;
 pub const TEST_CALLEE_ACCOUNT_UPDATES: u8 = 41;
 pub const TEST_STACK_HEAP_ZEROED: u8 = 42;
+pub const TEST_ACCOUNT_INFO_IN_ACCOUNT: u8 = 43;
 
 pub const MINT_INDEX: usize = 0;
 pub const ARGUMENT_INDEX: usize = 1;

programs/sbf/tests/programs.rs

@@ -4929,6 +4929,78 @@ fn test_update_callee_account() {
     }
 }
 
+#[test]
+fn test_account_info_in_account() {
+    solana_logger::setup();
+
+    let GenesisConfigInfo {
+        genesis_config,
+        mint_keypair,
+        ..
+    } = create_genesis_config(100_123_456_789);
+
+    let mut programs = Vec::new();
+    #[cfg(feature = "sbf_c")]
+    {
+        programs.push("invoke");
+    }
+    #[cfg(feature = "sbf_rust")]
+    {
+        programs.push("solana_sbf_rust_invoke");
+    }
+
+    for program in programs {
+        for direct_mapping in [false, true] {
+            let mut bank = Bank::new_for_tests(&genesis_config);
+            let feature_set = Arc::make_mut(&mut bank.feature_set);
+            // by default test banks have all features enabled, so we only need to
+            // disable when needed
+            if !direct_mapping {
+                feature_set.deactivate(&feature_set::bpf_account_data_direct_mapping::id());
+            }
+
+            let (bank, bank_forks) = bank.wrap_with_bank_forks_for_tests();
+            let mut bank_client = BankClient::new_shared(bank.clone());
+            let authority_keypair = Keypair::new();
+
+            let (bank, invoke_program_id) = load_upgradeable_program_and_advance_slot(
+                &mut bank_client,
+                bank_forks.as_ref(),
+                &mint_keypair,
+                &authority_keypair,
+                program,
+            );
+
+            let account_keypair = Keypair::new();
+
+            let mint_pubkey = mint_keypair.pubkey();
+
+            let account_metas = vec![
+                AccountMeta::new(mint_pubkey, true),
+                AccountMeta::new(account_keypair.pubkey(), false),
+                AccountMeta::new_readonly(invoke_program_id, false),
+            ];
+
+            let mut instruction_data = vec![TEST_ACCOUNT_INFO_IN_ACCOUNT];
+            instruction_data.extend_from_slice(32usize.to_le_bytes().as_ref());
+
+            let instruction =
+                Instruction::new_with_bytes(invoke_program_id, &instruction_data, account_metas);
+
+            let account = AccountSharedData::new(42, 10240, &invoke_program_id);
+
+            bank.store_account(&account_keypair.pubkey(), &account);
+
+            let result = bank_client.send_and_confirm_instruction(&mint_keypair, instruction);
+            if direct_mapping {
+                assert!(result.is_err());
+            } else {
+                assert!(result.is_ok());
+            }
+        }
+    }
+}
+
 #[test]
 fn test_clone_account_data() {
     // Test cloning account data works as expect with