Check account boundaries on overlapping memmove syscall (#4563)

* fix reverse memmove

Author: seanyoung

programs/bpf_loader/src/syscalls/mem_ops.rs

@@ -419,7 +419,7 @@ struct MemoryChunkIterator<'a> {
     // exclusive end index (start + len, so one past the last valid address)
     vm_addr_end: u64,
     len: u64,
-    account_index: usize,
+    account_index: Option<usize>,
     is_account: Option<bool>,
 }
 
@@ -446,7 +446,7 @@ impl<'a> MemoryChunkIterator<'a> {
             len,
             vm_addr_start: vm_addr,
             vm_addr_end,
-            account_index: 0,
+            account_index: None,
             is_account: None,
         })
     }
@@ -490,14 +490,18 @@ impl<'a> Iterator for MemoryChunkIterator<'a> {
 
         let region_is_account;
 
+        let mut account_index = self.account_index.unwrap_or_default();
+        self.account_index = Some(account_index);
+
         loop {
-            if let Some(account) = self.accounts.get(self.account_index) {
+            if let Some(account) = self.accounts.get(account_index) {
                 let account_addr = account.vm_data_addr;
                 let resize_addr = account_addr.saturating_add(account.original_data_len as u64);
 
                 if resize_addr < region.vm_addr {
                     // region is after this account, move on next one
-                    self.account_index = self.account_index.saturating_add(1);
+                    account_index = account_index.saturating_add(1);
+                    self.account_index = Some(account_index);
                 } else {
                     region_is_account =
                         region.vm_addr == account_addr || region.vm_addr == resize_addr;
@@ -550,6 +554,41 @@ impl DoubleEndedIterator for MemoryChunkIterator<'_> {
             }
         };
 
+        let region_is_account;
+
+        let mut account_index = self
+            .account_index
+            .unwrap_or_else(|| self.accounts.len().saturating_sub(1));
+        self.account_index = Some(account_index);
+
+        loop {
+            let Some(account) = self.accounts.get(account_index) else {
+                // address is after all the accounts
+                region_is_account = false;
+                break;
+            };
+
+            let account_addr = account.vm_data_addr;
+            let resize_addr = account_addr.saturating_add(account.original_data_len as u64);
+
+            if account_index > 0 && account_addr > region.vm_addr {
+                account_index = account_index.saturating_sub(1);
+
+                self.account_index = Some(account_index);
+            } else {
+                region_is_account = region.vm_addr == account_addr || region.vm_addr == resize_addr;
+                break;
+            }
+        }
+
+        if let Some(is_account) = self.is_account {
+            if is_account != region_is_account {
+                return Some(Err(SyscallError::InvalidLength.into()));
+            }
+        } else {
+            self.is_account = Some(region_is_account);
+        }
+
         let chunk_len = if region.vm_addr >= self.vm_addr_start {
             // consume the whole region
             let len = self.vm_addr_end.saturating_sub(region.vm_addr);

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

@@ -108,7 +108,20 @@ pub fn process_instruction(
             // memmov dst overlaps begin of account
             unsafe { sol_memmove(too_early(3).as_mut_ptr(), buf.as_ptr(), 10) };
         }
-
+        14 => {
+            // memmove dst overlaps begin of account, reverse order
+            unsafe { sol_memmove(too_early(0).as_mut_ptr(), too_early(3).as_ptr(), 10) };
+        }
+        15 => {
+            // memmove dst overlaps end of account, reverse order
+            unsafe {
+                sol_memmove(
+                    data[data_len..].as_mut_ptr(),
+                    data[data_len.saturating_sub(3)..].as_mut_ptr(),
+                    10,
+                )
+            };
+        }
         _ => {}
     }
 

programs/sbf/tests/programs.rs

@@ -5644,7 +5644,7 @@ fn test_mem_syscalls_overlap_account_begin_or_end() {
         let account = AccountSharedData::new(42, 1024, &program_id);
         bank.store_account(&account_keypair.pubkey(), &account);
 
-        for instr in 0..=13 {
+        for instr in 0..=15 {
             println!("Testing direct_mapping:{direct_mapping} instruction:{instr}");
             let instruction =
                 Instruction::new_with_bytes(program_id, &[instr], account_metas.clone());

sdk/feature-set/src/lib.rs

@@ -617,7 +617,7 @@ pub mod apply_cost_tracker_during_replay {
     solana_pubkey::declare_id!("2ry7ygxiYURULZCrypHhveanvP5tzZ4toRwVp89oCNSj");
 }
 pub mod bpf_account_data_direct_mapping {
-    solana_pubkey::declare_id!("GJVDwRkUPNdk9QaK4VsU4g1N41QNxhy1hevjf8kz45Mq");
+    solana_pubkey::declare_id!("FNPWmNbHbYy1R8JWVZgCPqsoRBcRu4F6ezSnq5o97Px");
 }
 
 pub mod add_set_tx_loaded_accounts_data_size_instruction {