From ed95dfe01988224103daa32ee20f50454d168366 Mon Sep 17 00:00:00 2001 From: onspeedhp Date: Fri, 6 Feb 2026 21:21:00 +0700 Subject: [PATCH 1/2] fix(security): validation skips discriminator check (Issue #7) - validate wallet discriminator (must be 1) in create_session.rs - validate wallet discriminator in manage_authority.rs (add/remove) - validate wallet discriminator in execute.rs - validate wallet discriminator in transfer_ownership.rs --- program/src/processor/create_session.rs | 6 ++++++ program/src/processor/execute.rs | 7 ++++++- program/src/processor/manage_authority.rs | 11 +++++++++++ program/src/processor/transfer_ownership.rs | 5 +++++ tests-e2e/TEST_ISSUES.md | 4 ++++ 5 files changed, 32 insertions(+), 1 deletion(-) diff --git a/program/src/processor/create_session.rs b/program/src/processor/create_session.rs index 68e8df7..6f2065f 100644 --- a/program/src/processor/create_session.rs +++ b/program/src/processor/create_session.rs @@ -110,6 +110,12 @@ pub fn process( return Err(ProgramError::IllegalOwner); } + // Validate Wallet Discriminator (Issue #7) + let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() }; + if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 { + return Err(ProgramError::InvalidAccountData); + } + // Verify Authorizer // Check removed: conditional writable check inside match diff --git a/program/src/processor/execute.rs b/program/src/processor/execute.rs index e50eecb..a16e949 100644 --- a/program/src/processor/execute.rs +++ b/program/src/processor/execute.rs @@ -4,7 +4,7 @@ use crate::{ }, compact::parse_compact_instructions, error::AuthError, - state::authority::AuthorityAccountHeader, + state::{authority::AuthorityAccountHeader, AccountDiscriminator}, }; use pinocchio::{ account_info::AccountInfo, @@ -61,6 +61,11 @@ pub fn process( if wallet_pda.owner() != program_id || authority_pda.owner() != program_id { return Err(ProgramError::IllegalOwner); } + // Validate Wallet Discriminator (Issue #7) + let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() }; + if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 { + return Err(ProgramError::InvalidAccountData); + } if !authority_pda.is_writable() { return Err(ProgramError::InvalidAccountData); diff --git a/program/src/processor/manage_authority.rs b/program/src/processor/manage_authority.rs index 0201844..6b88ab7 100644 --- a/program/src/processor/manage_authority.rs +++ b/program/src/processor/manage_authority.rs @@ -133,6 +133,17 @@ pub fn process_add_authority( if admin_auth_pda.owner() != program_id { return Err(ProgramError::IllegalOwner); } + // Validate Wallet Discriminator (Issue #7) + let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() }; + if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 { + return Err(ProgramError::InvalidAccountData); + } + + // Validate Wallet Discriminator (Issue #7) + let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() }; + if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 { + return Err(ProgramError::InvalidAccountData); + } // Validate system_program is the correct System Program (audit N2) if !sol_assert_bytes_eq( diff --git a/program/src/processor/transfer_ownership.rs b/program/src/processor/transfer_ownership.rs index 0a89d47..d9714b7 100644 --- a/program/src/processor/transfer_ownership.rs +++ b/program/src/processor/transfer_ownership.rs @@ -111,6 +111,11 @@ pub fn process( if wallet_pda.owner() != program_id || current_owner.owner() != program_id { return Err(ProgramError::IllegalOwner); } + // Validate Wallet Discriminator (Issue #7) + let wallet_data = unsafe { wallet_pda.borrow_data_unchecked() }; + if wallet_data.is_empty() || wallet_data[0] != AccountDiscriminator::Wallet as u8 { + return Err(ProgramError::InvalidAccountData); + } // Validate system_program is the correct System Program (audit N2) if !sol_assert_bytes_eq( diff --git a/tests-e2e/TEST_ISSUES.md b/tests-e2e/TEST_ISSUES.md index 591cd15..1446477 100644 --- a/tests-e2e/TEST_ISSUES.md +++ b/tests-e2e/TEST_ISSUES.md @@ -30,6 +30,10 @@ **Status**: ✅ Fixed **Fix**: Replaced hardcoded rent calculations with `Rent::minimum_balance(space)` in `create_wallet.rs` and `manage_authority.rs`. Verified by tests. +### Issue #8 (Validation): Wallet Discriminator Check +**Status**: ✅ Fixed +**Fix**: Added `wallet_data[0] == AccountDiscriminator::Wallet` check in `create_session.rs`, `manage_authority.rs`, `execute.rs`, and `transfer_ownership.rs`. + ## Current Status All E2E scenarios are PASSING. - Happy Path From 622022bd47d1edaf1700c89bb2f3fcb110b464af Mon Sep 17 00:00:00 2001 From: onspeedhp Date: Fri, 6 Feb 2026 21:23:08 +0700 Subject: [PATCH 2/2] test(e2e): add scenario 6 to verify wallet discriminator validation - Add new test case in failures.rs that attempts to use an Authority PDA (valid owner, wrong discriminator) as a Wallet PDA - Verify that the transaction is rejected with InvalidAccountData --- tests-e2e/TEST_ISSUES.md | 3 +- tests-e2e/src/scenarios/failures.rs | 51 +++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 1 deletion(-) diff --git a/tests-e2e/TEST_ISSUES.md b/tests-e2e/TEST_ISSUES.md index 1446477..66af958 100644 --- a/tests-e2e/TEST_ISSUES.md +++ b/tests-e2e/TEST_ISSUES.md @@ -33,11 +33,12 @@ ### Issue #8 (Validation): Wallet Discriminator Check **Status**: ✅ Fixed **Fix**: Added `wallet_data[0] == AccountDiscriminator::Wallet` check in `create_session.rs`, `manage_authority.rs`, `execute.rs`, and `transfer_ownership.rs`. +**Verification**: Added `Scenario 6: Wallet Discriminator Check` in `failures.rs`. Tested passing Authority PDA as Wallet PDA (Rejected). ## Current Status All E2E scenarios are PASSING. - Happy Path -- Failures (5/5) +- Failures (6/6) - Cross Wallet (3/3) - DoS Attack - Audit Validations diff --git a/tests-e2e/src/scenarios/failures.rs b/tests-e2e/src/scenarios/failures.rs index 876a482..7408bb2 100644 --- a/tests-e2e/src/scenarios/failures.rs +++ b/tests-e2e/src/scenarios/failures.rs @@ -360,5 +360,56 @@ pub fn run(ctx: &mut TestContext) -> Result<()> { ctx.execute_tx_expect_error(remove_owner_tx)?; println!("✅ Admin Removing Owner Rejected."); + // Scenario 6: Wallet Discriminator Check (Issue #7) + // Attempt to use an Authority PDA (owned by program, but wrong discriminator) as the Wallet PDA + println!("\n[6/6] Testing Wallet Discriminator Validation..."); + + // We will try to call CreateSession using the Owner Authority PDA as the "Wallet PDA" + // The Owner Authority PDA is owned by the program, so it passes the owner check. + // However, it has Discriminator::Authority (2), not Wallet (1), so it should fail the new check. + + let fake_wallet_pda = owner_auth_pda; // This is actually an Authority account + + let bad_session_keypair = Keypair::new(); + let (bad_session_pda, _) = Pubkey::find_program_address( + &[ + b"session", + fake_wallet_pda.as_ref(), // Derived from the "fake" wallet + bad_session_keypair.pubkey().as_ref(), + ], + &ctx.program_id, + ); + + let mut bad_session_data = Vec::new(); + bad_session_data.push(5); // CreateSession + bad_session_data.extend_from_slice(bad_session_keypair.pubkey().as_ref()); + bad_session_data.extend_from_slice(&(current_slot + 100).to_le_bytes()); + + let bad_discriminator_ix = Instruction { + program_id: ctx.program_id.to_address(), + accounts: vec![ + AccountMeta::new(Signer::pubkey(&ctx.payer).to_address(), true), + AccountMeta::new_readonly(fake_wallet_pda.to_address(), false), // FAKE WALLET (Authority Account) + AccountMeta::new_readonly(owner_auth_pda.to_address(), false), // Authorizer (Using same account as auth is technically weird but valid for this test) + AccountMeta::new(bad_session_pda.to_address(), false), + AccountMeta::new_readonly(solana_system_program::id().to_address(), false), + AccountMeta::new_readonly(solana_sysvar::rent::ID.to_address(), false), + AccountMeta::new_readonly(Signer::pubkey(&owner_keypair).to_address(), true), + ], + data: bad_session_data, + }; + + let message = Message::new( + &[bad_discriminator_ix], + Some(&Signer::pubkey(&ctx.payer).to_address()), + ); + let mut bad_disc_tx = Transaction::new_unsigned(message); + bad_disc_tx.sign(&[&ctx.payer, &owner_keypair], ctx.svm.latest_blockhash()); + + // Expect InvalidAccountData (which is often generic error or custom depending on implementation) + // Our fix returns InvalidAccountData + ctx.execute_tx_expect_error(bad_disc_tx)?; + println!("✅ Invalid Wallet Discriminator Rejected."); + Ok(()) }