diff --git a/artifacts/test_program_methods/malicious_injector.bin b/artifacts/test_program_methods/malicious_injector.bin new file mode 100644 index 00000000..40e57b81 Binary files /dev/null and b/artifacts/test_program_methods/malicious_injector.bin differ diff --git a/artifacts/test_program_methods/malicious_launderer.bin b/artifacts/test_program_methods/malicious_launderer.bin new file mode 100644 index 00000000..cdae83a3 Binary files /dev/null and b/artifacts/test_program_methods/malicious_launderer.bin differ diff --git a/nssa/src/program.rs b/nssa/src/program.rs index 1aff3bc9..c3c92f1e 100644 --- a/nssa/src/program.rs +++ b/nssa/src/program.rs @@ -469,6 +469,24 @@ mod tests { use test_program_methods::PINATA_COOLDOWN_ELF; Self::new(PINATA_COOLDOWN_ELF.to_vec()).unwrap() } + + #[must_use] + pub fn malicious_injector() -> Self { + use test_program_methods::{MALICIOUS_INJECTOR_ELF, MALICIOUS_INJECTOR_ID}; + Self { + id: MALICIOUS_INJECTOR_ID, + elf: MALICIOUS_INJECTOR_ELF.to_vec(), + } + } + + #[must_use] + pub fn malicious_launderer() -> Self { + use test_program_methods::{MALICIOUS_LAUNDERER_ELF, MALICIOUS_LAUNDERER_ID}; + Self { + id: MALICIOUS_LAUNDERER_ID, + elf: MALICIOUS_LAUNDERER_ELF.to_vec(), + } + } } #[test] diff --git a/nssa/src/validated_state_diff.rs b/nssa/src/validated_state_diff.rs index 4bd5fb05..5deef6a4 100644 --- a/nssa/src/validated_state_diff.rs +++ b/nssa/src/validated_state_diff.rs @@ -265,7 +265,7 @@ impl ValidatedStateDiff { state_diff.insert(pre.account_id, post.account().clone()); } - let authorized_accounts: HashSet<_> = chained_call + let authorized_accounts: HashSet<_> = program_output .pre_states .iter() .filter(|pre| pre.is_authorized) @@ -488,3 +488,113 @@ fn n_unique(data: &[T]) -> usize { let set: HashSet<&T> = data.iter().collect(); set.len() } + +#[cfg(test)] +mod tests { + use nssa_core::account::{AccountId, Nonce}; + + use crate::{ + PrivateKey, PublicKey, V03State, + program::Program, + public_transaction::{Message, WitnessSet}, + validated_state_diff::ValidatedStateDiff, + }; + + /// Demonstrates the authorization-injection vulnerability: + /// two malicious programs (injector + launderer) drain a victim's balance + /// without the victim signing anything. + /// + /// Attack flow: + /// Transaction (attacker signs) → P1 (`malicious_injector`) + /// → injects `victim(is_authorized=true)` into chained call `pre_states` for P2 + /// P2 (`malicious_launderer`) + /// → outputs empty pre/post states (victim never checked against authorized set) + /// → `authorized_accounts` for `authenticated_transfer` built from + /// `program_output.pre_states` = {victim} `authenticated_transfer` + /// → `victim.is_authorized=true` passes check ({victim}.contains(victim)) + /// → transfer executes. + #[test] + fn malicious_programs_drain_victim_without_signature() { + // p2_id, auth_transfer_id, victim_id_raw, victim_balance, victim_nonce, + // victim_program_owner, recipient_id_raw, amount. + // Primitives only — AccountId/Account cannot round-trip through instruction_data + // via risc0_zkvm::serde (SerializeDisplay issue). + type InjectorInstruction = ( + nssa_core::program::ProgramId, // p2_id + nssa_core::program::ProgramId, // auth_transfer_id + [u8; 32], // victim_id_raw + u128, // victim_balance + u128, // victim_nonce + nssa_core::program::ProgramId, // victim_program_owner + [u8; 32], // recipient_id_raw + u128, // amount + ); + + let attacker_key = PrivateKey::try_new([10; 32]).unwrap(); + let attacker_id = AccountId::from(&PublicKey::new_from_private_key(&attacker_key)); + + let victim_key = PrivateKey::try_new([20; 32]).unwrap(); + let victim_id = AccountId::from(&PublicKey::new_from_private_key(&victim_key)); + + let recipient_id = AccountId::new([42; 32]); + + let victim_balance = 5_000_u128; + let mut state = V03State::new_with_genesis_accounts( + &[ + (attacker_id, 100), + (victim_id, victim_balance), + (recipient_id, 0), + ], + vec![], + 0, + ); + + state.insert_program(Program::malicious_injector()); + state.insert_program(Program::malicious_launderer()); + + // Read victim state from chain, exactly as the attacker would. + let victim_account = state.get_account_by_id(victim_id); + + let instruction: InjectorInstruction = ( + Program::malicious_launderer().id(), + Program::authenticated_transfer_program().id(), + *victim_id.value(), + victim_account.balance, + victim_account.nonce.0, + victim_account.program_owner, + *recipient_id.value(), + victim_balance, + ); + + let message = Message::try_new( + Program::malicious_injector().id(), + vec![attacker_id], + vec![Nonce(0)], + instruction, + ) + .unwrap(); + + let witness_set = WitnessSet::for_message(&message, &[&attacker_key]); + let tx = crate::PublicTransaction::new(message, witness_set); + + let result = ValidatedStateDiff::from_public_transaction(&tx, &state, 1, 0); + + assert!( + result.is_err(), + "attack transaction should be rejected by the fixed validator" + ); + + // Confirm the victim's balance is untouched. + let victim_balance_after = state.get_account_by_id(victim_id).balance; + let recipient_balance_after = state.get_account_by_id(recipient_id).balance; + + assert_eq!( + victim_balance_after, victim_balance, + "victim balance should be unchanged" + ); + assert_eq!( + recipient_balance_after, 0, + "recipient should receive nothing" + ); + } +} diff --git a/test_program_methods/guest/src/bin/malicious_injector.rs b/test_program_methods/guest/src/bin/malicious_injector.rs new file mode 100644 index 00000000..4e7300a2 --- /dev/null +++ b/test_program_methods/guest/src/bin/malicious_injector.rs @@ -0,0 +1,105 @@ +use nssa_core::{ + account::{Account, AccountId, AccountWithMetadata, Data, Nonce}, + program::{ + AccountPostState, ChainedCall, ProgramId, ProgramInput, ProgramOutput, read_nssa_inputs, + }, +}; + +/// Instruction uses only risc0-serde-compatible primitives — no `AccountId`/`Account` structs, +/// which use `SerializeDisplay`/`DeserializeFromStr` and cannot round-trip through +/// `instruction_data`. +/// +/// Fields: +/// `p2_id`: program ID of the launderer (P2) +/// `auth_transfer_id`: program ID of `authenticated_transfer`, forwarded to P2 +/// `victim_id_raw`: raw `[u8; 32]` of the victim `AccountId` +/// `victim_balance`: victim's current balance +/// `victim_nonce`: victim's current nonce (inner `u128`) +/// `victim_program_owner`: victim account's `program_owner` field +/// `recipient_id_raw`: raw `[u8; 32]` of the recipient `AccountId` +/// `amount`: balance to transfer out of the victim. +type Instruction = ( + ProgramId, + ProgramId, + [u8; 32], + u128, + u128, + ProgramId, + [u8; 32], + u128, +); + +fn main() { + let ( + ProgramInput { + self_program_id, + caller_program_id, + pre_states, + instruction: + ( + p2_id, + auth_transfer_id, + victim_id_raw, + victim_balance, + victim_nonce, + victim_program_owner, + recipient_id_raw, + amount, + ), + }, + instruction_words, + ) = read_nssa_inputs::(); + + // Echo own pre_states (attacker's account) unchanged. + let post_states = pre_states + .iter() + .map(|p| AccountPostState::new(p.account.clone())) + .collect(); + + // Construct victim AccountWithMetadata from primitives, stamping is_authorized=true. + // Victim has not signed anything — this flag is forged entirely by P1's logic. + let victim = AccountWithMetadata { + account: Account { + program_owner: victim_program_owner, + balance: victim_balance, + data: Data::default(), + nonce: Nonce(victim_nonce), + }, + is_authorized: true, + account_id: AccountId::new(victim_id_raw), + }; + + // Recipient is already initialized under authenticated_transfer (program_owner = + // auth_transfer_id, balance = 0). Using the default account would trigger + // Claim::Authorized inside authenticated_transfer, which requires is_authorized=true + // on the recipient — a check that would block the transfer. + let recipient = AccountWithMetadata { + account: Account { + program_owner: auth_transfer_id, + balance: 0, + data: Data::default(), + nonce: Nonce(0), + }, + is_authorized: false, + account_id: AccountId::new(recipient_id_raw), + }; + + // Forward auth_transfer_id and amount to P2 so it can call authenticated_transfer. + let p2_instruction = risc0_zkvm::serde::to_vec(&(auth_transfer_id, amount)) + .expect("serialization is infallible"); + + ProgramOutput::new( + self_program_id, + caller_program_id, + instruction_words, + pre_states, + post_states, + ) + .with_chained_calls(vec![ChainedCall { + program_id: p2_id, + pre_states: vec![victim, recipient], + instruction_data: p2_instruction, + pda_seeds: vec![], + }]) + .write(); +} diff --git a/test_program_methods/guest/src/bin/malicious_launderer.rs b/test_program_methods/guest/src/bin/malicious_launderer.rs new file mode 100644 index 00000000..6d0568fd --- /dev/null +++ b/test_program_methods/guest/src/bin/malicious_launderer.rs @@ -0,0 +1,43 @@ +use nssa_core::program::{ChainedCall, ProgramId, ProgramInput, ProgramOutput, read_nssa_inputs}; + +/// Instruction: (`auth_transfer_id`, `amount`) — both primitive, safe for `risc0_zkvm::serde`. +type Instruction = (ProgramId, u128); + +fn main() { + let ( + ProgramInput { + self_program_id, + caller_program_id, + pre_states, + instruction: (auth_transfer_id, amount), + }, + instruction_words, + ) = read_nssa_inputs::(); + + // Output empty pre/post states. P2 processes no accounts itself, so the + // authorization check at validated_state_diff.rs:158-182 runs over nothing. + // Victim is never compared against caller_data.authorized_accounts = {attacker}. + // + // The bug: authorized_accounts for authenticated_transfer is built from + // chained_call.pre_states (this call's inputs, set by P1), which contains + // victim(is_authorized=true). So authorized_accounts = {victim}, and the + // subsequent check passes. + let auth_transfer_instruction = + risc0_zkvm::serde::to_vec(&authenticated_transfer_core::Instruction::Transfer { amount }) + .expect("serialization is infallible"); + + ProgramOutput::new( + self_program_id, + caller_program_id, + instruction_words, + vec![], + vec![], + ) + .with_chained_calls(vec![ChainedCall { + program_id: auth_transfer_id, + pre_states, + instruction_data: auth_transfer_instruction, + pda_seeds: vec![], + }]) + .write(); +}