Backport of #28599 to v1.14 (#28601)

* Extends is_nonoverlapping() to be able to deal with two different lengths.

* Uses is_nonoverlapping() for syscall output parameters.

* Feature gates the new throws of SyscallError::CopyOverlapping.

programs/bpf_loader/src/syscalls/mem_ops.rs

@@ -36,7 +36,7 @@ declare_syscall!(
             .feature_set
             .is_active(&check_physical_overlapping::id());
 
-        if !is_nonoverlapping(src_addr, dst_addr, n) {
+        if !is_nonoverlapping(src_addr, n, dst_addr, n) {
             *result = Err(SyscallError::CopyOverlapping.into());
             return;
         }
@@ -64,7 +64,7 @@ declare_syscall!(
         )
         .as_ptr();
         if do_check_physical_overlapping
-            && !is_nonoverlapping(src_ptr as usize, dst_ptr as usize, n as usize)
+            && !is_nonoverlapping(src_ptr as usize, n as usize, dst_ptr as usize, n as usize)
         {
             unsafe {
                 std::ptr::copy(src_ptr, dst_ptr, n as usize);

programs/bpf_loader/src/syscalls/mod.rs

@@ -34,7 +34,7 @@ use {
         entrypoint::{BPF_ALIGN_OF_U128, MAX_PERMITTED_DATA_INCREASE, SUCCESS},
         feature_set::{
             self, blake3_syscall_enabled, check_physical_overlapping, check_slice_translation_size,
-            curve25519_syscall_enabled, disable_fees_sysvar,
+            check_syscall_outputs_do_not_overlap, curve25519_syscall_enabled, disable_fees_sysvar,
             enable_early_verification_of_account_modifications, libsecp256k1_0_5_upgrade_enabled,
             limit_secp256k1_recovery_id, prevent_calling_precompiles_as_programs,
             syscall_saturated_math,
@@ -812,6 +812,18 @@ declare_syscall!(
                         ),
                         result
                     );
+                    if !is_nonoverlapping(
+                        bump_seed_ref as *const _ as usize,
+                        std::mem::size_of_val(bump_seed_ref),
+                        address.as_ptr() as usize,
+                        std::mem::size_of::<Pubkey>(),
+                    ) && invoke_context
+                        .feature_set
+                        .is_active(&check_syscall_outputs_do_not_overlap::id())
+                    {
+                        *result = Err(SyscallError::CopyOverlapping.into());
+                        return;
+                    }
                     *bump_seed_ref = bump_seed[0];
                     address.copy_from_slice(new_address.as_ref());
                     *result = Ok(0);
@@ -1681,6 +1693,19 @@ declare_syscall!(
                 result
             );
 
+            if !is_nonoverlapping(
+                to_slice.as_ptr() as usize,
+                length as usize,
+                program_id_result as *const _ as usize,
+                std::mem::size_of::<Pubkey>(),
+            ) && invoke_context
+                .feature_set
+                .is_active(&check_syscall_outputs_do_not_overlap::id())
+            {
+                *result = Err(SyscallError::CopyOverlapping.into());
+                return;
+            }
+
             *program_id_result = *program_id;
         }
 
@@ -1744,10 +1769,7 @@ declare_syscall!(
         };
 
         if let Some(instruction_context) = instruction_context {
-            let ProcessedSiblingInstruction {
-                data_len,
-                accounts_len,
-            } = question_mark!(
+            let result_header = question_mark!(
                 translate_type_mut::<ProcessedSiblingInstruction>(
                     memory_mapping,
                     meta_addr,
@@ -1756,8 +1778,8 @@ declare_syscall!(
                 result
             );
 
-            if *data_len == (instruction_context.get_instruction_data().len() as u64)
-                && *accounts_len
+            if result_header.data_len == (instruction_context.get_instruction_data().len() as u64)
+                && result_header.accounts_len
                     == (instruction_context.get_number_of_instruction_accounts() as u64)
             {
                 let program_id = question_mark!(
@@ -1772,7 +1794,7 @@ declare_syscall!(
                     translate_slice_mut::<u8>(
                         memory_mapping,
                         data_addr,
-                        *data_len as u64,
+                        result_header.data_len as u64,
                         invoke_context.get_check_aligned(),
                         invoke_context.get_check_size(),
                     ),
@@ -1782,13 +1804,54 @@ declare_syscall!(
                     translate_slice_mut::<AccountMeta>(
                         memory_mapping,
                         accounts_addr,
-                        *accounts_len as u64,
+                        result_header.accounts_len as u64,
                         invoke_context.get_check_aligned(),
                         invoke_context.get_check_size(),
                     ),
                     result
                 );
 
+                if (!is_nonoverlapping(
+                    result_header as *const _ as usize,
+                    std::mem::size_of::<ProcessedSiblingInstruction>(),
+                    program_id as *const _ as usize,
+                    std::mem::size_of::<Pubkey>(),
+                ) || !is_nonoverlapping(
+                    result_header as *const _ as usize,
+                    std::mem::size_of::<ProcessedSiblingInstruction>(),
+                    accounts.as_ptr() as usize,
+                    std::mem::size_of::<AccountMeta>()
+                        .saturating_mul(result_header.accounts_len as usize),
+                ) || !is_nonoverlapping(
+                    result_header as *const _ as usize,
+                    std::mem::size_of::<ProcessedSiblingInstruction>(),
+                    data.as_ptr() as usize,
+                    result_header.data_len as usize,
+                ) || !is_nonoverlapping(
+                    program_id as *const _ as usize,
+                    std::mem::size_of::<Pubkey>(),
+                    data.as_ptr() as usize,
+                    result_header.data_len as usize,
+                ) || !is_nonoverlapping(
+                    program_id as *const _ as usize,
+                    std::mem::size_of::<Pubkey>(),
+                    accounts.as_ptr() as usize,
+                    std::mem::size_of::<AccountMeta>()
+                        .saturating_mul(result_header.accounts_len as usize),
+                ) || !is_nonoverlapping(
+                    data.as_ptr() as usize,
+                    result_header.data_len as usize,
+                    accounts.as_ptr() as usize,
+                    std::mem::size_of::<AccountMeta>()
+                        .saturating_mul(result_header.accounts_len as usize),
+                )) && invoke_context
+                    .feature_set
+                    .is_active(&check_syscall_outputs_do_not_overlap::id())
+                {
+                    *result = Err(SyscallError::CopyOverlapping.into());
+                    return;
+                }
+
                 *program_id = *question_mark!(
                     instruction_context
                         .get_last_program_key(invoke_context.transaction_context)
@@ -1816,8 +1879,9 @@ declare_syscall!(
                 );
                 accounts.clone_from_slice(account_metas.as_slice());
             }
-            *data_len = instruction_context.get_instruction_data().len() as u64;
-            *accounts_len = instruction_context.get_number_of_instruction_accounts() as u64;
+            result_header.data_len = instruction_context.get_instruction_data().len() as u64;
+            result_header.accounts_len =
+                instruction_context.get_number_of_instruction_accounts() as u64;
             *result = Ok(true as u64);
             return;
         }

sdk/program/src/program_stubs.rs

@@ -54,7 +54,7 @@ pub trait SyscallStubs: Sync + Send {
     unsafe fn sol_memcpy(&self, dst: *mut u8, src: *const u8, n: usize) {
         // cannot be overlapping
         assert!(
-            is_nonoverlapping(src as usize, dst as usize, n),
+            is_nonoverlapping(src as usize, n, dst as usize, n),
             "memcpy does not support overlapping regions"
         );
         std::ptr::copy_nonoverlapping(src, dst, n as usize);
@@ -196,18 +196,20 @@ pub(crate) fn sol_get_stack_height() -> u64 {
 
 /// Check that two regions do not overlap.
 ///
-/// Adapted from libcore, hidden to share with bpf_loader without being part of
-/// the API surface.
+/// Hidden to share with bpf_loader without being part of the API surface.
 #[doc(hidden)]
-pub fn is_nonoverlapping<N>(src: N, dst: N, count: N) -> bool
+pub fn is_nonoverlapping<N>(src: N, src_len: N, dst: N, dst_len: N) -> bool
 where
     N: Ord + std::ops::Sub<Output = N>,
     <N as std::ops::Sub>::Output: Ord,
 {
-    let diff = if src > dst { src - dst } else { dst - src };
-    // If the absolute distance between the ptrs is at least as big as the size of the buffer,
+    // If the absolute distance between the ptrs is at least as big as the size of the other,
     // they do not overlap.
-    diff >= count
+    if src > dst {
+        src - dst >= dst_len
+    } else {
+        dst - src >= src_len
+    }
 }
 
 #[cfg(test)]
@@ -216,12 +218,16 @@ mod tests {
 
     #[test]
     fn test_is_nonoverlapping() {
-        assert!(is_nonoverlapping(10, 7, 3));
-        assert!(!is_nonoverlapping(10, 8, 3));
-        assert!(!is_nonoverlapping(10, 9, 3));
-        assert!(!is_nonoverlapping(10, 10, 3));
-        assert!(!is_nonoverlapping(10, 11, 3));
-        assert!(!is_nonoverlapping(10, 12, 3));
-        assert!(is_nonoverlapping(10, 13, 3));
+        for dst in 0..8 {
+            assert!(is_nonoverlapping(10, 3, dst, 3));
+        }
+        for dst in 8..13 {
+            assert!(!is_nonoverlapping(10, 3, dst, 3));
+        }
+        for dst in 13..20 {
+            assert!(is_nonoverlapping(10, 3, dst, 3));
+        }
+        assert!(is_nonoverlapping::<u8>(255, 3, 254, 1));
+        assert!(!is_nonoverlapping::<u8>(255, 2, 254, 3));
     }
 }

sdk/src/feature_set.rs

@@ -508,6 +508,10 @@ pub mod increase_tx_account_lock_limit {
     solana_sdk::declare_id!("9LZdXeKGeBV6hRLdxS1rHbHoEUsKqesCC2ZAPTPKJAbK");
 }
 
+pub mod check_syscall_outputs_do_not_overlap {
+    solana_sdk::declare_id!("3uRVPBpyEJRo1emLCrq38eLRFGcu6uKSpUXqGvU8T7SZ");
+}
+
 lazy_static! {
     /// Map of feature identifiers to user-visible description
     pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
@@ -629,6 +633,7 @@ lazy_static! {
         (vote_state_update_root_fix::id(), "fix root in vote state updates #27361"),
         (return_none_for_zero_lamport_accounts::id(), "return none for zero lamport accounts #27800"),
         (increase_tx_account_lock_limit::id(), "increase tx account lock limit to 128 #27241"),
+        (check_syscall_outputs_do_not_overlap::id(), "check syscall outputs do_not overlap #28600"),
         /*************** ADD NEW FEATURES HERE ***************/
     ]
     .iter()