feat: pinocchio swap example#18
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| pub fn parse(accounts: &'a [AccountInfo], params: &SwapParams) -> Result<Self, ProgramError> { | ||
| if accounts.len() < Self::FIXED_LEN { | ||
| return Err(ProgramError::NotEnoughAccountKeys); | ||
| } | ||
|
|
||
| let user = &accounts[0]; | ||
| let pool = &accounts[1]; | ||
| let pool_authority = &accounts[2]; | ||
| let mint_a = &accounts[3]; | ||
| let mint_b = &accounts[4]; | ||
| let vault_a = &accounts[5]; | ||
| let vault_b = &accounts[6]; | ||
| let user_token_a = &accounts[7]; | ||
| let user_token_b = &accounts[8]; | ||
| let light_token_program = &accounts[9]; | ||
| let light_token_cpi_authority = &accounts[10]; | ||
| let system_program = &accounts[11]; | ||
|
|
||
| // Validate user is signer | ||
| if !user.is_signer() { | ||
| return Err(ProgramError::MissingRequiredSignature); | ||
| } | ||
|
|
||
| // Validate global pool authority PDA | ||
| { | ||
| let seeds: &[&[u8]] = &[POOL_AUTHORITY_SEED]; | ||
| let (expected_pda, expected_bump) = | ||
| pinocchio::pubkey::find_program_address(seeds, &crate::ID); | ||
| if pool_authority.key() != &expected_pda { | ||
| return Err(SwapError::InvalidAuthoritySeeds.into()); | ||
| } | ||
| if expected_bump != params.authority_bump { | ||
| return Err(SwapError::InvalidAuthoritySeeds.into()); | ||
| } | ||
| } | ||
|
|
||
| Ok(Self { | ||
| user, | ||
| pool, | ||
| pool_authority, | ||
| mint_a, | ||
| mint_b, | ||
| vault_a, | ||
| vault_b, | ||
| user_token_a, | ||
| user_token_b, | ||
| light_token_program, | ||
| light_token_cpi_authority, | ||
| system_program, | ||
| }) |
There was a problem hiding this comment.
🔴 Missing pool account ownership and PDA validation in swap instruction enables fee manipulation
The swap instruction does not validate that the pool account is owned by the program or is a valid PDA derived from the mints.
Click to expand
Vulnerability Details
In swap/accounts.rs:55-104, the SwapAccounts::parse function validates the pool_authority PDA (lines 78-89) but does NOT validate the pool account:
- No check that
pool.owner() == &crate::ID - No check that
poolis a valid PDA derived from[POOL_SEED, mint_a, mint_b]
In swap/processor.rs:62-72, the processor validates that the provided vaults and mints match what's stored in the pool state, but an attacker controls what's in the pool state if they provide a fake pool account.
Attack Scenario
- Attacker creates a fake pool account (not owned by the program) with:
token_a_vaultandtoken_b_vaultpointing to legitimate vaults from an existing pooltoken_a_mintandtoken_b_mintmatching the legitimate poolfee_bps = 0(instead of the intended 30 bps)
- Attacker calls swap with the fake pool account and legitimate vaults
- Swap proceeds with 0% fee instead of the intended 0.3% fee
Since pool_authority is a global PDA ([POOL_AUTHORITY_SEED]) that owns all vaults across all pools, the transfer from vault will succeed because pool_authority legitimately owns the vault.
Impact
Fee bypass allowing users to swap without paying the intended pool fees, causing loss of protocol revenue.
Recommendation: Add validation in SwapAccounts::parse to verify the pool account is owned by the program and is a valid PDA derived from the mints:
// Validate pool is owned by this program
if pool.owner() != &crate::ID {
return Err(SwapError::InvalidPoolState.into());
}
// Validate pool PDA
let mint_a_key = mint_a.key();
let mint_b_key = mint_b.key();
let seeds: &[&[u8]] = &[POOL_SEED, mint_a_key.as_ref(), mint_b_key.as_ref()];
let (expected_pool_pda, _) = pinocchio::pubkey::find_program_address(seeds, &crate::ID);
if pool.key() != &expected_pool_pda {
return Err(SwapError::InvalidPoolSeeds.into());
}
Was this helpful? React with 👍 or 👎 to provide feedback.
| solana-pubkey = "2.2" | ||
| solana-instruction = "2.2" | ||
| solana-msg = "2.2" | ||
| solana-program-error = "2.2" |
There was a problem hiding this comment.
should not be necessary
2 participants
Uh oh!
There was an error while loading. Please reload this page.