Skip to content

Commit 4ee0a8a

Browse files
committed
fix(program): enforce canonical PDA validation on init
Restore canonical bump enforcement for instruction-provided PDA bumps while preserving a fast path for trusted stored bumps. Add integration regressions that verify noncanonical-but-valid PDA addresses are rejected for escrow, allowed mint, receipt, and extensions initialization flows.
1 parent 5b465d9 commit 4ee0a8a

7 files changed

Lines changed: 146 additions & 18 deletions

File tree

program/src/state/allowed_mint.rs

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,14 @@ impl AllowedMint {
6565
mint: &Address,
6666
) -> Result<&'a Self, ProgramError> {
6767
let state = Self::from_bytes(data)?;
68-
let pda = AllowedMintPda::new(escrow, mint);
69-
pda.validate_pda(account, program_id, state.bump)?;
68+
let derived = Address::derive_address(
69+
&[AllowedMintPda::PREFIX, escrow.as_ref(), mint.as_ref()],
70+
Some(state.bump),
71+
program_id,
72+
);
73+
if account.address() != &derived {
74+
return Err(ProgramError::InvalidSeeds);
75+
}
7076
Ok(state)
7177
}
7278
}

program/src/traits/pda.rs

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,33 @@ pub trait PdaSeeds {
2121
Address::find_program_address(&seeds, program_id)
2222
}
2323

24-
/// Validate that account matches derived PDA using `create_program_address` (~1500 CU).
24+
/// Validate that account matches the canonical PDA for these seeds.
2525
///
26-
/// Cheaper than `find_program_address` since bump is already known.
27-
/// Use `validate_self` instead when the bump is stored in the account.
26+
/// This enforces the canonical bump (highest valid bump) returned by
27+
/// `find_program_address`.
2828
#[inline(always)]
29-
fn validate_pda(&self, account: &AccountView, program_id: &Address, bump: u8) -> Result<(), ProgramError> {
29+
fn validate_pda(&self, account: &AccountView, program_id: &Address, expected_bump: u8) -> Result<(), ProgramError> {
30+
let (derived, bump) = self.derive_address(program_id);
31+
if bump != expected_bump {
32+
return Err(ProgramError::InvalidSeeds);
33+
}
34+
if account.address() != &derived {
35+
return Err(ProgramError::InvalidSeeds);
36+
}
37+
Ok(())
38+
}
39+
40+
/// Validate that account matches PDA derived with the provided bump.
41+
///
42+
/// This does not enforce canonical bump selection. Use `validate_pda`
43+
/// when canonical bump invariants must be enforced (e.g., account creation).
44+
#[inline(always)]
45+
fn validate_pda_with_bump(
46+
&self,
47+
account: &AccountView,
48+
program_id: &Address,
49+
bump: u8,
50+
) -> Result<(), ProgramError> {
3051
let seeds = self.seeds();
3152
let bump_arr = [bump];
3253
let mut seeds_with_bump = seeds;
@@ -62,7 +83,7 @@ pub trait PdaAccount: PdaSeeds {
6283
/// Validate that account matches derived PDA using stored bump
6384
#[inline(always)]
6485
fn validate_self(&self, account: &AccountView, program_id: &Address) -> Result<(), ProgramError> {
65-
self.validate_pda(account, program_id, self.bump())
86+
self.validate_pda_with_bump(account, program_id, self.bump())
6687
}
6788
}
6889

tests/integration-tests/src/test_add_timelock.rs

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,9 @@ use crate::{
22
fixtures::{AddTimelockFixture, CreateEscrowFixture, SetImmutableFixture},
33
utils::{
44
assert_escrow_error, assert_extensions_header, assert_instruction_error, assert_timelock_extension,
5-
find_escrow_pda, find_extensions_pda, test_empty_data, test_missing_signer, test_not_writable,
6-
test_truncated_data, test_wrong_account, test_wrong_current_program, test_wrong_system_program, EscrowError,
7-
InstructionTestFixture, TestContext, RANDOM_PUBKEY,
5+
find_escrow_pda, find_extensions_pda, find_noncanonical_program_address, test_empty_data, test_missing_signer,
6+
test_not_writable, test_truncated_data, test_wrong_account, test_wrong_current_program,
7+
test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext, RANDOM_PUBKEY,
88
},
99
};
1010
use solana_sdk::{instruction::InstructionError, signature::Signer};
@@ -53,6 +53,24 @@ fn test_add_timelock_invalid_extensions_bump() {
5353
assert_instruction_error(error, InstructionError::InvalidSeeds);
5454
}
5555

56+
#[test]
57+
fn test_add_timelock_noncanonical_extensions_pda_rejected() {
58+
let mut ctx = TestContext::new();
59+
let test_ix = AddTimelockFixture::build_valid(&mut ctx);
60+
let escrow = test_ix.instruction.accounts[2].pubkey;
61+
let program_id = test_ix.instruction.program_id;
62+
63+
let (noncanonical_extensions, noncanonical_bump) =
64+
find_noncanonical_program_address(&[b"extensions", escrow.as_ref()], &program_id)
65+
.expect("expected at least one noncanonical bump");
66+
67+
let error = test_ix
68+
.with_account_at(3, noncanonical_extensions)
69+
.with_data_byte_at(1, noncanonical_bump)
70+
.send_expect_error(&mut ctx);
71+
assert_instruction_error(error, InstructionError::InvalidSeeds);
72+
}
73+
5674
#[test]
5775
fn test_add_timelock_empty_data() {
5876
let mut ctx = TestContext::new();

tests/integration-tests/src/test_allow_mint.rs

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@ use crate::{
22
fixtures::{AllowMintFixture, AllowMintSetup},
33
utils::{
44
assert_account_exists, assert_allowed_mint_account, assert_escrow_error, assert_instruction_error,
5-
find_allowed_mint_pda, test_missing_signer, test_not_writable, test_wrong_current_program,
6-
test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext, RANDOM_PUBKEY,
5+
find_allowed_mint_pda, find_noncanonical_program_address, test_missing_signer, test_not_writable,
6+
test_wrong_current_program, test_wrong_system_program, EscrowError, InstructionTestFixture, TestContext,
7+
RANDOM_PUBKEY,
78
},
89
};
910
use escrow_program_client::instructions::{AllowMintBuilder, SetImmutableBuilder};
@@ -64,6 +65,26 @@ fn test_allow_mint_invalid_bump() {
6465
assert_instruction_error(error, InstructionError::InvalidSeeds);
6566
}
6667

68+
#[test]
69+
fn test_allow_mint_noncanonical_pda_rejected() {
70+
let mut ctx = TestContext::new();
71+
let setup = AllowMintSetup::new(&mut ctx);
72+
let valid_ix = setup.build_instruction(&ctx);
73+
let program_id = valid_ix.instruction.program_id;
74+
75+
let (noncanonical_allowed_mint, noncanonical_bump) = find_noncanonical_program_address(
76+
&[b"allowed_mint", setup.escrow_pda.as_ref(), setup.mint_pubkey.as_ref()],
77+
&program_id,
78+
)
79+
.expect("expected at least one noncanonical bump");
80+
81+
let error = valid_ix
82+
.with_account_at(5, noncanonical_allowed_mint)
83+
.with_data_byte_at(1, noncanonical_bump)
84+
.send_expect_error(&mut ctx);
85+
assert_instruction_error(error, InstructionError::InvalidSeeds);
86+
}
87+
6788
#[test]
6889
fn test_allow_mint_wrong_admin() {
6990
let mut ctx = TestContext::new();

tests/integration-tests/src/test_create_escrow.rs

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
use crate::{
22
fixtures::CreateEscrowFixture,
33
utils::{
4-
assert_escrow_account, assert_escrow_mutability, assert_instruction_error, test_empty_data,
5-
test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program,
4+
assert_escrow_account, assert_escrow_mutability, assert_instruction_error, find_noncanonical_program_address,
5+
test_empty_data, test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program,
66
test_wrong_system_program, InstructionTestFixture, TestContext,
77
},
88
};
@@ -70,6 +70,25 @@ fn test_create_escrow_invalid_bump() {
7070
assert_instruction_error(error, InstructionError::InvalidSeeds);
7171
}
7272

73+
#[test]
74+
fn test_create_escrow_noncanonical_bump_rejected() {
75+
let mut ctx = TestContext::new();
76+
let valid_ix = CreateEscrowFixture::build_valid(&mut ctx);
77+
78+
let escrow_seed = valid_ix.instruction.accounts[2].pubkey;
79+
let program_id = valid_ix.instruction.program_id;
80+
let (noncanonical_escrow, noncanonical_bump) =
81+
find_noncanonical_program_address(&[b"escrow", escrow_seed.as_ref()], &program_id)
82+
.expect("expected at least one noncanonical bump");
83+
84+
let error = valid_ix
85+
.with_account_at(3, noncanonical_escrow)
86+
.with_data_byte_at(1, noncanonical_bump)
87+
.send_expect_error(&mut ctx);
88+
89+
assert_instruction_error(error, InstructionError::InvalidSeeds);
90+
}
91+
7392
#[test]
7493
fn test_create_escrow_empty_data() {
7594
let mut ctx = TestContext::new();

tests/integration-tests/src/test_deposit.rs

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ use crate::{
44
DEFAULT_DEPOSIT_AMOUNT,
55
},
66
utils::{
7-
assert_custom_error, assert_escrow_error, assert_instruction_error, find_receipt_pda, test_empty_data,
8-
test_missing_signer, test_not_writable, test_wrong_account, test_wrong_current_program, test_wrong_owner,
9-
test_wrong_system_program, test_wrong_token_program, EscrowError, TestContext, TestInstruction,
10-
TEST_HOOK_ALLOW_ID, TEST_HOOK_DENY_ERROR, TEST_HOOK_DENY_ID,
7+
assert_custom_error, assert_escrow_error, assert_instruction_error, find_noncanonical_program_address,
8+
find_receipt_pda, test_empty_data, test_missing_signer, test_not_writable, test_wrong_account,
9+
test_wrong_current_program, test_wrong_owner, test_wrong_system_program, test_wrong_token_program, EscrowError,
10+
TestContext, TestInstruction, TEST_HOOK_ALLOW_ID, TEST_HOOK_DENY_ERROR, TEST_HOOK_DENY_ID,
1111
},
1212
};
1313
use escrow_program_client::instructions::DepositBuilder;
@@ -84,6 +84,29 @@ fn test_deposit_invalid_bump() {
8484
assert_instruction_error(error, InstructionError::InvalidSeeds);
8585
}
8686

87+
#[test]
88+
fn test_deposit_noncanonical_receipt_pda_rejected() {
89+
let mut ctx = TestContext::new();
90+
let setup = DepositSetup::new(&mut ctx);
91+
let valid_ix = setup.build_instruction(&ctx);
92+
let program_id = valid_ix.instruction.program_id;
93+
94+
let depositor = setup.depositor.pubkey();
95+
let mint = setup.mint.pubkey();
96+
let receipt_seed = setup.receipt_seed.pubkey();
97+
let (noncanonical_receipt, noncanonical_bump) = find_noncanonical_program_address(
98+
&[b"receipt", setup.escrow_pda.as_ref(), depositor.as_ref(), mint.as_ref(), receipt_seed.as_ref()],
99+
&program_id,
100+
)
101+
.expect("expected at least one noncanonical bump");
102+
103+
let error = valid_ix
104+
.with_account_at(5, noncanonical_receipt)
105+
.with_data_byte_at(1, noncanonical_bump)
106+
.send_expect_error(&mut ctx);
107+
assert_instruction_error(error, InstructionError::InvalidSeeds);
108+
}
109+
87110
#[test]
88111
fn test_deposit_wrong_token_program() {
89112
let mut ctx = TestContext::new();

tests/integration-tests/src/utils/pda_utils.rs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,3 +20,23 @@ pub fn find_receipt_pda(escrow: &Pubkey, depositor: &Pubkey, mint: &Pubkey, rece
2020
pub fn find_allowed_mint_pda(escrow: &Pubkey, mint: &Pubkey) -> (Pubkey, u8) {
2121
AllowedMint::find_pda(escrow, mint)
2222
}
23+
24+
pub fn find_noncanonical_program_address(seeds: &[&[u8]], program_id: &Pubkey) -> Option<(Pubkey, u8)> {
25+
let (_, canonical_bump) = Pubkey::find_program_address(seeds, program_id);
26+
27+
for bump in (0u8..=u8::MAX).rev() {
28+
if bump == canonical_bump {
29+
continue;
30+
}
31+
32+
let bump_seed = [bump];
33+
let mut seeds_with_bump = seeds.to_vec();
34+
seeds_with_bump.push(&bump_seed);
35+
36+
if let Ok(address) = Pubkey::create_program_address(&seeds_with_bump, program_id) {
37+
return Some((address, bump));
38+
}
39+
}
40+
41+
None
42+
}

0 commit comments

Comments
 (0)