fix(v2): re-check mut_mask on remaining_accounts walk

bench/programs/multisig/anchor-v2/src/lib.rs

@@ -16,7 +16,7 @@ pub mod multisig_v2 {
 
     #[discrim = 0]
     pub fn create(ctx: &mut Context<Create>, threshold: u8) -> Result<()> {
-        let remaining = ctx.remaining_accounts();
+        let remaining = ctx.remaining_accounts()?;
         ctx.accounts.create_multisig(threshold, &remaining)?;
         ctx.accounts.config.bump = ctx.bumps.config;
         Ok(())
@@ -34,7 +34,7 @@ pub mod multisig_v2 {
 
     #[discrim = 3]
     pub fn execute_transfer(ctx: &mut Context<ExecuteTransfer>, amount: u64) -> Result<()> {
-        let remaining = ctx.remaining_accounts();
+        let remaining = ctx.remaining_accounts()?;
         ctx.accounts.verify_and_transfer(amount, ctx.bumps.vault, &remaining)
     }
 }

lang-v2/Cargo.toml

@@ -98,3 +98,7 @@ required-features = ["testing"]
 [[test]]
 name = "cursor_and_remaining_accounts"
 required-features = ["testing"]
+
+[[test]]
+name = "remaining_accounts_mut_check"
+required-features = ["testing"]

lang-v2/src/context.rs

@@ -1,6 +1,7 @@
 use {
     crate::cursor::AccountCursor,
     pinocchio::{account::AccountView, address::Address},
+    solana_program_error::ProgramError,
 };
 
 /// Instruction-scoped context passed to every handler. Holds the
@@ -19,21 +20,39 @@ pub struct Context<'a, T: Bumps> {
     /// pass them in as arguments.
     pub bumps: T::Bumps,
 
-    /// Cursor into the serialized input buffer, positioned at the
+    /// Holds either a cursor into the serialized input buffer, pointing to the
     /// *start* of the remaining-accounts region (after `try_accounts`
     /// has consumed exactly `T::HEADER_SIZE` declared accounts). Used
     /// by [`Self::remaining_accounts`] for on-demand walking.
-    cursor: &'a mut AccountCursor,
+    /// After `remaining_accounts` is called this holds a cache of the result.
+    remaining_accounts: RemainingAccounts<'a>,
 
-    /// Number of accounts after the declared region. Decremented to
-    /// zero once the remaining region is walked.
-    remaining_num: u8,
+    /// Mutable-account mask covering the declared region (from
+    /// `T::MUT_MASK`). Used by [`Self::remaining_accounts`] to
+    /// re-check each trailing account against declared mut slots —
+    /// without this, a trailing account whose dup index points at a
+    /// mut declared account would silently alias it (bug 3: the
+    /// `HEADER_SIZE`-only check in `run_handler` can't see dups that
+    /// only surface during the trailing walk).
+    mut_mask: &'static [u64; 4],
+}
 
-    /// Cache of the parsed remaining accounts. Populated lazily on the
-    /// first call to [`Self::remaining_accounts`]; subsequent calls
-    /// return a clone of the cached vec so the caller receives an
-    /// owned value (avoiding borrow conflicts with `self.accounts`).
-    remaining_cache: Option<alloc::vec::Vec<AccountView>>,
+enum RemainingAccounts<'a> {
+    Unparsed {
+        /// Points to the `remaining-accounts` region
+        cursor: &'a mut AccountCursor,
+        /// Number of accounts remaining after the initially declared region
+        remaining: u8,
+    },
+    /// Cached result of walking `remaining-accounts`; will return an error if
+    /// duplicate mutable constraints are violated.
+    Cached(Result<alloc::vec::Vec<AccountView>, ProgramError>),
+}
+
+impl<'a> RemainingAccounts<'a> {
+    fn is_unparsed(&self) -> bool {
+        matches!(self, RemainingAccounts::Unparsed { .. })
+    }
 }
 
 impl<'a, T: Bumps> Context<'a, T> {
@@ -44,33 +63,74 @@ impl<'a, T: Bumps> Context<'a, T> {
         bumps: T::Bumps,
         cursor: &'a mut AccountCursor,
         remaining_num: u8,
+        mut_mask: &'static [u64; 4],
     ) -> Self {
         Self {
             program_id,
             accounts,
             bumps,
-            cursor,
-            remaining_num,
-            remaining_cache: None,
+            remaining_accounts: RemainingAccounts::Unparsed {
+                cursor,
+                remaining: remaining_num,
+            },
+            mut_mask,
         }
     }
 
     /// Returns trailing accounts beyond the declared `T` fields as an
-    /// owned `Vec<AccountView>`. First call walks the cursor and caches;
-    /// subsequent calls clone the cache. Owned vec avoids borrow conflicts
-    /// with `self.accounts` / `self.bumps`.
-    pub fn remaining_accounts(&mut self) -> alloc::vec::Vec<AccountView> {
-        if self.remaining_cache.is_none() {
-            let mut v = alloc::vec::Vec::with_capacity(self.remaining_num as usize);
-            for _ in 0..self.remaining_num {
-                // SAFETY: cursor is positioned at the start of the
-                // remaining region and `remaining_num` is the exact
-                // number of accounts to walk.
-                v.push(unsafe { self.cursor.next() });
+    /// owned `Vec<AccountView>`. First call walks the cursor and caches
+    /// the resulting `Result`; subsequent calls replay the cache (clone
+    /// of the vec on success, clone of the error on failure). Caching
+    /// the error is important: the walk advances an unsafe cursor, and
+    /// a handler that calls this again after an error must not trigger
+    /// another `cursor.next()` loop.
+    ///
+    /// After each cursor advance, re-tests the cursor's duplicate bitvec
+    /// against `T::MUT_MASK`. If a trailing account's dup index resolves
+    /// to a declared mut slot, returns
+    /// `ConstraintDuplicateMutableAccount`. The `HEADER_SIZE`-only check
+    /// in `run_handler` only sees duplicates that existed at the end of
+    /// the declared walk; trailing-region dups can only be caught here.
+    ///
+    /// `MUT_MASK` is sized per declared field, so bits set for trailing
+    /// indices (past `HEADER_SIZE`) are naturally zero — the intersect
+    /// only fires when a trailing slot's bit overlaps with a declared
+    /// mut slot's bit, which by construction means the runtime resolved
+    /// the trailing slot as a dup of that declared mut account.
+    pub fn remaining_accounts(&mut self) -> Result<&alloc::vec::Vec<AccountView>, ProgramError> {
+        if self.remaining_accounts.is_unparsed() {
+            self.remaining_accounts = RemainingAccounts::Cached(self.walk_remaining());
+        }
+
+        match &self.remaining_accounts {
+            RemainingAccounts::Cached(Ok(accs)) => Ok(accs),
+            RemainingAccounts::Cached(Err(err)) => Err(err.clone()),
+            RemainingAccounts::Unparsed { .. } => unreachable!(),
+        }
+    }
+
+    fn walk_remaining(&mut self) -> Result<alloc::vec::Vec<AccountView>, ProgramError> {
+        let RemainingAccounts::Unparsed {
+            ref mut cursor,
+            remaining,
+        } = self.remaining_accounts
+        else {
+            unreachable!()
+        };
+        let mut v = alloc::vec::Vec::with_capacity(remaining as usize);
+        for _ in 0..remaining {
+            // SAFETY: cursor is positioned at the start of the remaining
+            // region and `remaining_num` is the exact number of accounts
+            // to walk. Walking stops on the first mut-alias error so we
+            // never advance past the remaining region.
+            v.push(unsafe { cursor.next() });
+            if let Some(dups) = cursor.duplicates() {
+                if dups.intersects(self.mut_mask) {
+                    return Err(crate::ErrorCode::ConstraintDuplicateMutableAccount.into());
+                }
             }
-            self.remaining_cache = Some(v);
         }
-        self.remaining_cache.as_ref().unwrap().clone()
+        Ok(v)
     }
 }
 

lang-v2/src/cursor.rs

@@ -82,6 +82,18 @@ impl AccountCursor {
         self.consumed
     }
 
+    /// Current duplicate-tracking bitvec. `None` if the cursor has not
+    /// yet yielded a duplicate account (lazy allocation — see
+    /// [`Self::next`]). Used by
+    /// [`Context::remaining_accounts`](crate::context::Context::remaining_accounts)
+    /// to re-check `MUT_MASK` after each trailing account is walked,
+    /// catching aliases of declared mut accounts that only surface past
+    /// `HEADER_SIZE`.
+    #[inline(always)]
+    pub fn duplicates(&self) -> Option<&AccountBitvec> {
+        self.duplicate.as_ref()
+    }
+
     /// Walk N accounts in a tight loop, storing views in the lookup array.
     /// Returns a slice of the walked views and the duplicate tracking bitvec.
     /// This avoids interleaving cursor math with validation logic, letting

lang-v2/src/dispatch.rs

@@ -85,7 +85,14 @@ pub fn run_handler<'a, T: TryAccounts>(
         T::try_accounts(program_id, views, duplicates, 0, ix_data)?
     };
     let remaining_num = (num_accounts - T::HEADER_SIZE) as u8;
-    let mut ctx = Context::new(program_id, ctx_accounts, bumps, cursor, remaining_num);
+    let mut ctx = Context::new(
+        program_id,
+        ctx_accounts,
+        bumps,
+        cursor,
+        remaining_num,
+        &T::MUT_MASK,
+    );
     handler(&mut ctx, ix_args)?;
     ctx.accounts.exit_accounts()?;
     Ok(())

lang-v2/tests/cursor_and_remaining_accounts.rs

@@ -38,6 +38,11 @@ impl Bumps for DummyHeader {
     type Bumps = ();
 }
 
+/// No declared mut fields — these tests don't exercise the
+/// trailing-dup-vs-declared-mut check (that's covered by
+/// `remaining_accounts_mut_check.rs`).
+const EMPTY_MUT_MASK: &[u64; 4] = &[0, 0, 0, 0];
+
 fn unique_addr(i: u8) -> [u8; 32] {
     let mut a = [0u8; 32];
     a[0] = i + 1; // avoid [0;32] which collides with the System program id.
@@ -198,9 +203,10 @@ fn remaining_accounts_walks_trailing_region() {
         (),
         &mut cursor,
         /*remaining_num*/ 3,
+        EMPTY_MUT_MASK,
     );
 
-    let remaining = ctx.remaining_accounts();
+    let remaining = ctx.remaining_accounts().expect("walk");
     assert_eq!(remaining.len(), 3);
     assert_eq!(remaining[0].address().to_bytes(), unique_addr(2));
     assert_eq!(remaining[1].address().to_bytes(), unique_addr(3));
@@ -216,11 +222,11 @@ fn remaining_accounts_returns_empty_when_nothing_trails() {
 
     let program_id = Address::new_from_array([0x42; 32]);
     let mut ctx: Context<'_, DummyHeader> =
-        Context::new(&program_id, DummyHeader, (), &mut cursor, 0);
+        Context::new(&program_id, DummyHeader, (), &mut cursor, 0, EMPTY_MUT_MASK);
 
-    assert!(ctx.remaining_accounts().is_empty());
+    assert!(ctx.remaining_accounts().expect("walk").is_empty());
     // Second call on empty — still empty, no cache bookkeeping bug.
-    assert!(ctx.remaining_accounts().is_empty());
+    assert!(ctx.remaining_accounts().expect("walk").is_empty());
 }
 
 #[test]
@@ -241,13 +247,17 @@ fn remaining_accounts_caches_and_does_not_re_walk_cursor() {
         (),
         &mut cursor,
         /*remaining_num*/ 2,
+        EMPTY_MUT_MASK,
     );
 
-    let first = ctx.remaining_accounts();
-    let second = ctx.remaining_accounts();
+    // `remaining_accounts()` returns `Result<&Vec<_>, _>`; the borrow
+    // would conflict with a second call, so clone-out the first vec
+    // before re-borrowing `ctx`.
+    let first = ctx.remaining_accounts().cloned().expect("first walk");
+    let second = ctx.remaining_accounts().cloned().expect("second walk");
 
     // Structural equality via address, since AccountView is Copy and the
-    // cache returns a clone each call.
+    // cache returns the same vec each call.
     assert_eq!(first.len(), 2);
     assert_eq!(second.len(), 2);
     for (a, b) in first.iter().zip(second.iter()) {