diff --git a/Cargo.lock b/Cargo.lock index 8b47c991..68664480 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8103,7 +8103,7 @@ dependencies = [ [[package]] name = "pallet-shielded-pool" -version = "0.16.0" +version = "0.17.0" dependencies = [ "ark-bn254", "ark-ff 0.5.0", diff --git a/frame/shielded-pool/CHANGELOG.md b/frame/shielded-pool/CHANGELOG.md index 6c00ae4c..a5ac0713 100644 --- a/frame/shielded-pool/CHANGELOG.md +++ b/frame/shielded-pool/CHANGELOG.md @@ -2,6 +2,20 @@ All notable changes to `pallet-shielded-pool` will be documented in this file. +## [0.17.0] - 2026-08-07 + +### Security + +- **`private_transfer` rejects two equal non-dummy nullifiers.** Both slots + holding the same real nullifier would spend one input twice within a single + transfer: each clears the used-set check (neither is marked yet), and the + second `mark_as_used` is idempotent, so nothing downstream caught it. Pool + admission does not catch it either — `validate_unsigned` collapses equal + nullifiers into one `provides` tag rather than flagging the duplicate. The + proof is expected to bind the two inputs distinct, but the chain no longer + relies on that: `execute` now compares the non-dummy nullifiers directly (at + most two inputs) and refuses a duplicate with `NullifierAlreadyUsed`. + ## [0.16.0] - 2026-08-07 ### Security diff --git a/frame/shielded-pool/Cargo.toml b/frame/shielded-pool/Cargo.toml index 9b0521c1..ae846927 100644 --- a/frame/shielded-pool/Cargo.toml +++ b/frame/shielded-pool/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-shielded-pool" -version = "0.16.0" +version = "0.17.0" description = "Shielded pool pallet for private transactions using ZK proofs" authors = ["Orbinum Team"] license = "GPL-3.0-or-later" diff --git a/frame/shielded-pool/src/operations/private_transfer.rs b/frame/shielded-pool/src/operations/private_transfer.rs index 28b3d8f1..7832b206 100644 --- a/frame/shielded-pool/src/operations/private_transfer.rs +++ b/frame/shielded-pool/src/operations/private_transfer.rs @@ -60,6 +60,15 @@ impl PrivateTransferOperation { ); } + let non_dummy: sp_std::vec::Vec<&Nullifier> = + nullifiers.iter().filter(|n| n.0 != [0u8; 32]).collect(); + if non_dummy.len() == 2 { + ensure!( + non_dummy[0] != non_dummy[1], + Error::::NullifierAlreadyUsed + ); + } + for commitment in commitments.iter() { ensure!(commitment.is_canonical(), Error::::InvalidPublicSignals); ensure!(commitment.is_valid(), Error::::InvalidPublicSignals); @@ -301,6 +310,30 @@ mod tests { }); } + #[test] + fn execute_two_equal_nullifiers_fails() { + new_test_ext().execute_with(|| { + MerkleRepository::add_historic_poseidon_root::(KNOWN_ROOT); + + // The same non-dummy nullifier in both slots would spend one input + // twice: neither is in the used set yet, so both clear that check. + assert_noop!( + PrivateTransferOperation::execute::( + proof(), + KNOWN_ROOT, + nullifiers_of(&[0x20, 0x20]), + commitments_of(&[0x30, 0x31]), + memos_of(2), + 0u32, + 0u128, + None, + 1, + ), + crate::pallet::Error::::NullifierAlreadyUsed + ); + }); + } + #[test] fn execute_memo_commitment_mismatch_fails() { new_test_ext().execute_with(|| {