fix: reject multiple family members under same (program, seed) in one tx

This commit is contained in:
Moudy 2026-04-17 15:36:20 +02:00
parent f9a5a7635e
commit d3577f02bc
40 changed files with 174 additions and 31 deletions

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

Binary file not shown.

View File

@ -27,7 +27,7 @@ pub struct ProgramInput<T> {
/// Each program can derive up to `2^256` unique account IDs by choosing different /// Each program can derive up to `2^256` unique account IDs by choosing different
/// seeds. PDAs allow programs to control namespaced account identifiers without /// seeds. PDAs allow programs to control namespaced account identifiers without
/// collisions between programs. /// collisions between programs.
#[derive(Debug, Clone, Copy, Eq, PartialEq, Serialize, Deserialize)] #[derive(Debug, Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize)]
pub struct PdaSeed([u8; 32]); pub struct PdaSeed([u8; 32]);
impl PdaSeed { impl PdaSeed {

View File

@ -312,6 +312,16 @@ mod tests {
} }
} }
#[must_use]
pub fn two_pda_claimer() -> Self {
use test_program_methods::{TWO_PDA_CLAIMER_ELF, TWO_PDA_CLAIMER_ID};
Self {
id: TWO_PDA_CLAIMER_ID,
elf: TWO_PDA_CLAIMER_ELF.to_vec(),
}
}
#[must_use] #[must_use]
pub fn changer_claimer() -> Self { pub fn changer_claimer() -> Self {
use test_program_methods::{CHANGER_CLAIMER_ELF, CHANGER_CLAIMER_ID}; use test_program_methods::{CHANGER_CLAIMER_ELF, CHANGER_CLAIMER_ID};

View File

@ -2494,6 +2494,42 @@ pub mod tests {
/// `(seed, original_owner_program_id)` side input per mask-3 `pre_state` so the circuit /// `(seed, original_owner_program_id)` side input per mask-3 `pre_state` so the circuit
/// can re-verify `private_pda_account_id(owner, seed, npk) == pre.account_id` without a /// can re-verify `private_pda_account_id(owner, seed, npk) == pre.account_id` without a
/// claim. /// claim.
/// Exploit-scenario pin. A single `(program_id, seed)` pair can derive a family of
/// `AccountId`s (one public PDA and one private PDA per distinct npk). Without the tx-wide
/// family-binding check, a program could claim `PDA_alice` (mask-3, `alice_npk`) and
/// `PDA_bob` (mask-3, `bob_npk`) under the same seed in one transaction, and — once reuse
/// is supported — a later chained call could delegate both to a callee via
/// `pda_seeds: [S]` and mix balances across them. The binding check rejects the setup
/// here: after the first claim records `(program, seed) → PDA_alice`, the second claim
/// tries to record `(program, seed) → PDA_bob` and panics.
#[test]
fn two_mask_3_claims_under_same_seed_are_rejected() {
let program = Program::two_pda_claimer();
let keys_a = test_private_account_keys_1();
let keys_b = test_private_account_keys_2();
let seed = PdaSeed::new([55; 32]);
let shared_a = SharedSecretKey::new(&[66; 32], &keys_a.vpk());
let shared_b = SharedSecretKey::new(&[77; 32], &keys_b.vpk());
let account_a = private_pda_account_id(&program.id(), &seed, &keys_a.npk());
let account_b = private_pda_account_id(&program.id(), &seed, &keys_b.npk());
let pre_a = AccountWithMetadata::new(Account::default(), false, account_a);
let pre_b = AccountWithMetadata::new(Account::default(), false, account_b);
let result = execute_and_prove(
vec![pre_a, pre_b],
Program::serialize_instruction(seed).unwrap(),
vec![3, 3],
vec![(keys_a.npk(), shared_a), (keys_b.npk(), shared_b)],
vec![],
vec![None, None],
&program.into(),
);
assert!(matches!(result, Err(NssaError::CircuitProvingError(_))));
}
#[test] #[test]
fn mask_3_reuse_across_txs_currently_unsupported() { fn mask_3_reuse_across_txs_currently_unsupported() {
let program = Program::noop(); let program = Program::noop();
@ -2502,8 +2538,8 @@ pub mod tests {
let shared_secret = SharedSecretKey::new(&[55; 32], &keys.vpk()); let shared_secret = SharedSecretKey::new(&[55; 32], &keys.vpk());
let seed = PdaSeed::new([99; 32]); let seed = PdaSeed::new([99; 32]);
// Simulate a previously-claimed private PDA: program_owner != DEFAULT, is_authorized = true, // Simulate a previously-claimed private PDA: program_owner != DEFAULT, is_authorized =
// account_id derived via the private formula. // true, account_id derived via the private formula.
let account_id = private_pda_account_id(&program.id(), &seed, &npk); let account_id = private_pda_account_id(&program.id(), &seed, &npk);
let owned_pre_state = AccountWithMetadata::new( let owned_pre_state = AccountWithMetadata::new(
Account { Account {

View File

@ -11,7 +11,7 @@ use nssa_core::{
compute_digest_for_path, compute_digest_for_path,
program::{ program::{
AccountPostState, BlockValidityWindow, ChainedCall, Claim, DEFAULT_PROGRAM_ID, AccountPostState, BlockValidityWindow, ChainedCall, Claim, DEFAULT_PROGRAM_ID,
MAX_NUMBER_CHAINED_CALLS, ProgramId, ProgramOutput, TimestampValidityWindow, MAX_NUMBER_CHAINED_CALLS, PdaSeed, ProgramId, ProgramOutput, TimestampValidityWindow,
private_pda_account_id, validate_execution, private_pda_account_id, validate_execution,
}, },
}; };
@ -30,6 +30,14 @@ struct ExecutionState {
/// main loop, every mask-3 position must appear in this set; otherwise the npk is unbound /// main loop, every mask-3 position must appear in this set; otherwise the npk is unbound
/// and the circuit rejects. /// and the circuit rejects.
mask3_bound_positions: HashSet<usize>, mask3_bound_positions: HashSet<usize>,
/// Across the whole transaction, each `(program_id, seed)` pair may resolve to at most one
/// `AccountId`. A seed under a program can derive a family of accounts (one public PDA and
/// one private PDA per distinct npk), and unifying `Claim::PrivatePda` and `ChainedCall`'s
/// private seeds into plain `Claim::Pda(seed)` / `pda_seeds` would otherwise let a single
/// `pda_seeds: [S]` in a chained call authorize multiple family members at once. We record
/// every claim and caller-authorization resolution here and reject any mismatch, making the
/// rule: one `(program, seed)` → one account per tx.
pda_family_binding: HashMap<(ProgramId, PdaSeed), AccountId>,
} }
impl ExecutionState { impl ExecutionState {
@ -75,6 +83,7 @@ impl ExecutionState {
block_validity_window, block_validity_window,
timestamp_validity_window, timestamp_validity_window,
mask3_bound_positions: HashSet::new(), mask3_bound_positions: HashSet::new(),
pda_family_binding: HashMap::new(),
}; };
let Some(first_output) = program_outputs.first() else { let Some(first_output) = program_outputs.first() else {
@ -150,17 +159,12 @@ impl ExecutionState {
chained_calls.push_front((next_call.clone(), Some(chained_call.program_id))); chained_calls.push_front((next_call.clone(), Some(chained_call.program_id)));
} }
let authorized_public_pdas = nssa_core::program::compute_authorized_pdas(
caller_program_id,
&chained_call.pda_seeds,
);
execution_state.validate_and_sync_states( execution_state.validate_and_sync_states(
visibility_mask, visibility_mask,
mask3_npk_by_position, mask3_npk_by_position,
chained_call.program_id, chained_call.program_id,
caller_program_id, caller_program_id,
&chained_call.pda_seeds, &chained_call.pda_seeds,
&authorized_public_pdas,
program_output.pre_states, program_output.pre_states,
program_output.post_states, program_output.post_states,
); );
@ -212,15 +216,17 @@ impl ExecutionState {
} }
/// Validate program pre and post states and populate the execution state. /// Validate program pre and post states and populate the execution state.
#[expect(clippy::too_many_arguments, reason = "breaking out a context struct does not buy us anything here")] #[expect(
clippy::too_many_arguments,
reason = "breaking out a context struct does not buy us anything here"
)]
fn validate_and_sync_states( fn validate_and_sync_states(
&mut self, &mut self,
visibility_mask: &[u8], visibility_mask: &[u8],
mask3_npk_by_position: &HashMap<usize, NullifierPublicKey>, mask3_npk_by_position: &HashMap<usize, NullifierPublicKey>,
program_id: ProgramId, program_id: ProgramId,
caller_program_id: Option<ProgramId>, caller_program_id: Option<ProgramId>,
caller_pda_seeds: &[nssa_core::program::PdaSeed], caller_pda_seeds: &[PdaSeed],
authorized_public_pdas: &HashSet<AccountId>,
pre_states: Vec<AccountWithMetadata>, pre_states: Vec<AccountWithMetadata>,
post_states: Vec<AccountPostState>, post_states: Vec<AccountPostState>,
) { ) {
@ -259,28 +265,42 @@ impl ExecutionState {
|(pos, acc)| (acc.is_authorized, pos) |(pos, acc)| (acc.is_authorized, pos)
); );
let authorized_via_public = authorized_public_pdas.contains(&pre_account_id); // Find which caller seed (if any) authorizes this pre_state, under the
// Mask-3 PDAs are authorized by matching a caller seed against the private // public or the private derivation. We need the *specific* seed (not just a
// derivation with this pre_state's npk. The equality check binds the npk. // bool) so we can record the `(caller, seed) → account_id` family binding.
// Only reachable when `caller_program_id.is_some()` — top-level flows have // Only reachable when `caller_program_id.is_some()` — top-level flows have
// no caller-emitted seeds, so binding at top level must come from the // no caller-emitted seeds, so binding at top level must come from the claim
// claim path below. // path below.
let authorized_via_private = mask3_npk_by_position let matched_caller_seed: Option<(PdaSeed, bool)> =
.get(&pre_state_position) caller_program_id.and_then(|caller| {
.and_then(|npk| { caller_pda_seeds.iter().find_map(|seed| {
let caller = caller_program_id?; if AccountId::from((&caller, seed)) == pre_account_id {
caller_pda_seeds.iter().find(|seed| { return Some((*seed, false));
private_pda_account_id(&caller, seed, npk) == pre_account_id }
})?; if let Some(npk) = mask3_npk_by_position.get(&pre_state_position)
Some(()) && private_pda_account_id(&caller, seed, npk) == pre_account_id
}) {
.is_some(); return Some((*seed, true));
if authorized_via_private { }
self.mask3_bound_positions.insert(pre_state_position); None
})
});
if let Some((seed, is_private_form)) = matched_caller_seed {
let caller = caller_program_id
.expect("matched caller seed implies caller_program_id is set");
assert_family_binding(
&mut self.pda_family_binding,
caller,
seed,
pre_account_id,
);
if is_private_form {
self.mask3_bound_positions.insert(pre_state_position);
}
} }
let is_authorized = let is_authorized = previous_is_authorized || matched_caller_seed.is_some();
previous_is_authorized || authorized_via_public || authorized_via_private;
assert_eq!( assert_eq!(
pre_is_authorized, is_authorized, pre_is_authorized, is_authorized,
@ -324,6 +344,12 @@ impl ExecutionState {
pre_account_id, pda, pre_account_id, pda,
"Invalid PDA claim for account {pre_account_id} which does not match derived PDA {pda}" "Invalid PDA claim for account {pre_account_id} which does not match derived PDA {pda}"
); );
assert_family_binding(
&mut self.pda_family_binding,
program_id,
seed,
pre_account_id,
);
} }
}, },
3 => match claim { 3 => match claim {
@ -343,6 +369,12 @@ impl ExecutionState {
"Invalid private PDA claim for account {pre_account_id}" "Invalid private PDA claim for account {pre_account_id}"
); );
self.mask3_bound_positions.insert(pre_state_position); self.mask3_bound_positions.insert(pre_state_position);
assert_family_binding(
&mut self.pda_family_binding,
program_id,
seed,
pre_account_id,
);
} }
}, },
_ => { _ => {
@ -373,6 +405,34 @@ impl ExecutionState {
} }
} }
/// Record or re-verify the `(program_id, seed) → account_id` family binding for the
/// transaction. Any claim or caller-seed authorization that resolves a `pre_state` under
/// `(program_id, seed)` must agree with every prior resolution of the same pair; otherwise a
/// single `pda_seeds: [seed]` entry could authorize multiple private-PDA family members at
/// once (different npks under the same seed) and let a callee mix balances across them. Free
/// function so callers can pass `&mut self.pda_family_binding` without holding a borrow on
/// the surrounding struct's other fields.
fn assert_family_binding(
bindings: &mut HashMap<(ProgramId, PdaSeed), AccountId>,
program_id: ProgramId,
seed: PdaSeed,
account_id: AccountId,
) {
match bindings.entry((program_id, seed)) {
Entry::Vacant(e) => {
e.insert(account_id);
}
Entry::Occupied(e) => {
assert_eq!(
*e.get(),
account_id,
"Two different accounts resolved under the same (program, seed) in one transaction: existing {}, new {account_id}",
e.get()
);
}
}
}
fn compute_circuit_output( fn compute_circuit_output(
execution_state: ExecutionState, execution_state: ExecutionState,
visibility_mask: &[u8], visibility_mask: &[u8],

View File

@ -0,0 +1,37 @@
use nssa_core::program::{
AccountPostState, Claim, PdaSeed, ProgramInput, ProgramOutput, read_nssa_inputs,
};
/// Claims two `pre_states` under the same `seed`. Used to exercise the tx-wide
/// `(program_id, seed) → AccountId` family-binding check: when both `pre_states` are mask-3
/// with different npks, each `Claim::Pda(seed)` resolves to a different `AccountId` under the
/// same `(program, seed)` key, and the circuit must reject.
type Instruction = PdaSeed;
fn main() {
let (
ProgramInput {
self_program_id,
caller_program_id,
pre_states,
instruction: seed,
},
instruction_words,
) = read_nssa_inputs::<Instruction>();
let Ok([pre_a, pre_b]) = <[_; 2]>::try_from(pre_states) else {
return;
};
let claim_a = AccountPostState::new_claimed(pre_a.account.clone(), Claim::Pda(seed));
let claim_b = AccountPostState::new_claimed(pre_b.account.clone(), Claim::Pda(seed));
ProgramOutput::new(
self_program_id,
caller_program_id,
instruction_words,
vec![pre_a, pre_b],
vec![claim_a, claim_b],
)
.write();
}