multisig dup signer attack should-fail test

Author: rustopian

p-ata/benches/common_builders.rs

@@ -92,6 +92,8 @@ pub enum FailureMode {
     RecoverWalletNotSigner,
     /// Recover: multisig insufficient signers
     RecoverMultisigInsufficientSigners,
+    /// Recover: multisig duplicate signers (vulnerability test)
+    RecoverMultisigDuplicateSigners,
     /// Recover: wrong nested ATA address
     RecoverWrongNestedAta(Pubkey),
     /// Recover: wrong destination address
@@ -939,6 +941,10 @@ impl CommonTestCaseBuilder {
                     }
                 }
             }
+            FailureMode::RecoverMultisigDuplicateSigners => {
+                // This will be handled by the custom builder in failure_scenarios.rs
+                // The custom builder will duplicate a signer account to exploit the vulnerability
+            }
 
             // Address replacement (both instruction and accounts)
             FailureMode::WrongSystemProgram(wrong_id) => {

p-ata/benches/failure_scenarios.rs

@@ -168,6 +168,14 @@ static FAILURE_TESTS: &[FailureTestConfig] = &[
         failure_mode: FailureMode::RecoverMultisigInsufficientSigners,
         builder_type: TestBuilderType::Simple,
     },
+    FailureTestConfig {
+        name: "fail_recover_multisig_duplicate_signers",
+        category: TestCategory::RecoveryOperations,
+        base_test: BaseTestType::RecoverMultisig,
+        variant: TestVariant::BASE,
+        failure_mode: FailureMode::RecoverMultisigDuplicateSigners,
+        builder_type: TestBuilderType::Custom,
+    },
     FailureTestConfig {
         name: "fail_recover_wrong_nested_ata_address",
         category: TestCategory::RecoveryOperations,
@@ -409,6 +417,12 @@ impl FailureTestBuilder {
                     "fail_create_extended_mint_v1" => {
                         Self::build_fail_create_extended_mint_v1(ata_impl, token_program_id)
                     }
+                    "fail_recover_multisig_duplicate_signers" => {
+                        Self::build_fail_recover_multisig_duplicate_signers(
+                            ata_impl,
+                            token_program_id,
+                        )
+                    }
                     _ => panic!("Unknown custom test: {}", config.name),
                 }
             }
@@ -735,6 +749,43 @@ impl FailureTestBuilder {
         }
         (ix, accounts)
     }
+
+    /// Custom builder for multisig duplicate signers vulnerability test
+    /// This test exploits the vulnerability where the same signer can be counted multiple times
+    fn build_fail_recover_multisig_duplicate_signers(
+        ata_impl: &AtaImplementation,
+        token_program_id: &Pubkey,
+    ) -> (Instruction, Vec<(Pubkey, Account)>) {
+        // Start with a standard RecoverMultisig test case
+        let (mut ix, mut accounts) = CommonTestCaseBuilder::build_test_case(
+            BaseTestType::RecoverMultisig,
+            TestVariant::BASE,
+            ata_impl,
+            token_program_id,
+        );
+
+        log_test_info(
+            "fail_recover_multisig_duplicate_signers",
+            ata_impl,
+            &[("wallet", &ix.accounts[5].pubkey)],
+        );
+
+        // The standard RecoverMultisig test creates a 2-of-3 multisig
+        // We'll exploit the vulnerability by providing the same signer twice
+        // This should allow us to bypass the 2-of-3 requirement with only 1 actual signer
+
+        // Find the first signer account (should be at index 7 in the instruction accounts)
+        if ix.accounts.len() > 7 {
+            let first_signer = ix.accounts[7].pubkey;
+            ix.accounts
+                .push(AccountMeta::new_readonly(first_signer, true));
+            if let Some(existing_meta) = ix.accounts.get_mut(7) {
+                existing_meta.is_signer = true;
+            }
+        }
+
+        (ix, accounts)
+    }
 }
 
 // ================================ FAILURE TEST COMPARISON RUNNER ================================

p-ata/src/processor.rs

@@ -172,7 +172,7 @@ fn parse_create_accounts(accounts: &[AccountInfo]) -> Result<CreateAccounts, Pro
         _ => return Err(ProgramError::NotEnoughAccountKeys),
     };
 
-    // SAFETY: account info already checked
+    // SAFETY: account len already checked
     unsafe {
         Ok(CreateAccounts {
             payer: accounts.get_unchecked(0),