v1.17: cpi: fix capacity check in update_caller_account (backport of #…

…34064) (#34081) cpi: fix capacity check in update_caller_account (#34064) reserve(additional) reserves additional bytes on top of the current _length_ not capacity. Before this fix we could potentially reserve less capacity than required. (cherry picked from commit d009d73) Co-authored-by: Alessandro Decina <[email protected]>
solana-labs · Nov 15, 2023 · b767847 · b767847
1 parent 353aa64
commit b767847
Show file tree

Hide file tree

Showing 3 changed files with 40 additions and 6 deletions.
diff --git a/programs/bpf_loader/src/syscalls/cpi.rs b/programs/bpf_loader/src/syscalls/cpi.rs
@@ -1470,7 +1470,8 @@ fn update_caller_account(
             // invalid address.
             let min_capacity = caller_account.original_data_len;
             if callee_account.capacity() < min_capacity {
-                callee_account.reserve(min_capacity.saturating_sub(callee_account.capacity()))?;
+                callee_account
+                    .reserve(min_capacity.saturating_sub(callee_account.get_data().len()))?;
                 zero_all_mapped_spare_capacity = true;
             }
 

diff --git a/programs/sbf/rust/invoke/src/processor.rs b/programs/sbf/rust/invoke/src/processor.rs
@@ -1286,6 +1286,33 @@ fn process_instruction(
                 },
                 &vec![0; original_data_len - new_len]
             );
+
+            // Realloc to [0xFC; 2]
+            invoke(
+                &create_instruction(
+                    *callee_program_id,
+                    &[
+                        (accounts[ARGUMENT_INDEX].key, true, false),
+                        (callee_program_id, false, false),
+                    ],
+                    vec![0xFC; 2],
+                ),
+                accounts,
+            )
+            .unwrap();
+
+            // Check that [2..20] is zeroed
+            let new_len = account.data_len();
+            assert_eq!(&*account.data.borrow(), &[0xFC; 2]);
+            assert_eq!(
+                unsafe {
+                    slice::from_raw_parts(
+                        account.data.borrow().as_ptr().add(new_len),
+                        original_data_len - new_len,
+                    )
+                },
+                &vec![0; original_data_len - new_len]
+            );
         }
         TEST_WRITE_ACCOUNT => {
             msg!("TEST_WRITE_ACCOUNT");

diff --git a/programs/sbf/tests/programs.rs b/programs/sbf/tests/programs.rs
@@ -4413,12 +4413,18 @@ fn test_cpi_change_account_data_memory_allocation() {
 
         // Test changing the account data both in place and by changing the
         // underlying vector. CPI will have to detect the vector change and
-        // update the corresponding memory region. In both cases CPI will have
+        // update the corresponding memory region. In all cases CPI will have
         // to zero the spare bytes correctly.
-        if instruction_data[0] == 0xFE {
-            account.set_data(instruction_data.to_vec());
-        } else {
-            account.set_data_from_slice(instruction_data);
+        match instruction_data[0] {
+            0xFE => account.set_data(instruction_data.to_vec()),
+            0xFD => account.set_data_from_slice(instruction_data),
+            0xFC => {
+                // Exercise the update_caller_account capacity check where account len != capacity.
+                let mut data = instruction_data.to_vec();
+                data.reserve_exact(1);
+                account.set_data(data)
+            }
+            _ => panic!(),
         }
 
         Ok(())