contract/dao: prevent proposal input reuse

src/contract/dao/proof/propose-input.zk

@@ -15,6 +15,8 @@ witness "ProposeInput" {
     Base coin_user_data,
     Base coin_blind,
 
+    Base proposal_bulla,
+
     Scalar value_blind,
     Base coin_token_blind,
 
@@ -52,6 +54,14 @@ circuit "ProposeInput" {
     );
     constrain_instance(null_tree_root);
 
+    # Include some secret information in input nullifier to defeat
+    # input reuse attacks. We reveal the proposal_bulla in
+    # propose-main.zk as well. Always use different nullifiers for
+    # same inputs to avoid deanonimizing them between calls.
+    input_nullifier = poseidon_hash(nullifier, proposal_bulla);
+    constrain_instance(proposal_bulla);
+    constrain_instance(input_nullifier);
+
     # Pedersen commitment for coin's coin_value
     vcv = ec_mul_short(coin_value, VALUE_COMMIT_VALUE);
     vcr = ec_mul(value_blind, VALUE_COMMIT_RANDOM);

src/contract/dao/src/client/propose.rs

@@ -70,6 +70,11 @@ impl<T: StorageAdapter<Value = pallas::Base>> DaoProposeCall<'_, T> {
         main_zkbin: &ZkBinary,
         main_pk: &ProvingKey,
     ) -> Result<(DaoProposeParams, Vec<Proof>, Vec<SecretKey>)> {
+        if self.dao.to_bulla() != self.proposal.dao_bulla {
+            return Err(ClientFailed::VerifyError(DaoError::InvalidCalls.to_string()).into())
+        }
+        let proposal_bulla = self.proposal.to_bulla();
+
         let mut proofs = vec![];
         let mut signature_secrets = vec![];
 
@@ -119,6 +124,7 @@ impl<T: StorageAdapter<Value = pallas::Base>> DaoProposeCall<'_, T> {
                 Witness::Base(Value::known(pallas::Base::ZERO)),
                 Witness::Base(Value::known(pallas::Base::ZERO)),
                 Witness::Base(Value::known(note.coin_blind.inner())),
+                Witness::Base(Value::known(proposal_bulla.inner())),
                 Witness::Scalar(Value::known(funds_blind.inner())),
                 Witness::Base(Value::known(gov_token_blind.inner())),
                 Witness::Uint32(Value::known(leaf_pos.try_into().unwrap())),
@@ -151,8 +157,12 @@ impl<T: StorageAdapter<Value = pallas::Base>> DaoProposeCall<'_, T> {
             let value_commit = pedersen_commitment_u64(note.value, funds_blind);
             let value_coords = value_commit.to_affine().coordinates().unwrap();
 
+            let input_nullifier = poseidon_hash([nullifier, proposal_bulla.inner()]);
+
             let public_inputs = vec![
                 smt_null_root,
+                proposal_bulla.inner(),
+                input_nullifier,
                 *value_coords.x(),
                 *value_coords.y(),
                 token_commit,
@@ -172,6 +182,7 @@ impl<T: StorageAdapter<Value = pallas::Base>> DaoProposeCall<'_, T> {
                 value_commit,
                 merkle_coin_root,
                 smt_null_root,
+                input_nullifier: input_nullifier.into(),
                 signature_public,
             };
             inputs.push(input);
@@ -196,11 +207,6 @@ impl<T: StorageAdapter<Value = pallas::Base>> DaoProposeCall<'_, T> {
 
         let dao_leaf_position: u64 = self.dao_leaf_position.into();
 
-        if self.dao.to_bulla() != self.proposal.dao_bulla {
-            return Err(ClientFailed::VerifyError(DaoError::InvalidCalls.to_string()).into())
-        }
-        let proposal_bulla = self.proposal.to_bulla();
-
         let prover_witnesses = vec![
             // Proposers total number of gov tokens
             Witness::Base(Value::known(total_funds)),

src/contract/dao/src/entrypoint/propose.rs

@@ -75,6 +75,8 @@ pub(crate) fn dao_propose_get_metadata(
             DAO_CONTRACT_ZKAS_PROPOSE_INPUT_NS.to_string(),
             vec![
                 input.smt_null_root,
+                params.proposal_bulla.inner(),
+                input.input_nullifier.inner(),
                 *value_coords.x(),
                 *value_coords.y(),
                 params.token_commit,
@@ -123,8 +125,15 @@ pub(crate) fn dao_propose_process_instruction(
     let coin_roots_db = wasm::db::db_lookup(*MONEY_CONTRACT_ID, MONEY_CONTRACT_COIN_ROOTS_TREE)?;
     let null_roots_db =
         wasm::db::db_lookup(*MONEY_CONTRACT_ID, MONEY_CONTRACT_NULLIFIER_ROOTS_TREE)?;
+    let mut input_nullifiers = vec![];
 
     for input in &params.inputs {
+        // Check input has not been reused
+        if input_nullifiers.contains(&input.input_nullifier) {
+            msg!("[Dao::Propose] Error: Attempted to reuse input");
+            return Err(DaoError::ProposalInputsReuse.into())
+        }
+
         // Check the Merkle roots for the input coins are valid
         let Some(coin_root_data) =
             wasm::db::db_get(coin_roots_db, &serialize(&input.merkle_coin_root))?
@@ -174,11 +183,15 @@ pub(crate) fn dao_propose_process_instruction(
 
         // Check snapshot age againts current height
         let current_height = wasm::util::get_verifying_block_height()?;
+        // We assert here to prevent underflow and catch catastrophic
+        // internal failure.
+        assert!(current_height >= tx_height);
         if current_height - tx_height > PROPOSAL_SNAPSHOT_CUTOFF_LIMIT {
             msg!("[Dao::Propose] Error: Snapshot is too old. Current height: {}, snapshot height: {}",
                  current_height, tx_height);
             return Err(DaoError::SnapshotTooOld.into())
         }
+        input_nullifiers.push(input.input_nullifier);
     }
 
     // Is the DAO bulla generated in the ZK proof valid

src/contract/dao/src/error.rs

@@ -29,6 +29,9 @@ pub enum DaoError {
     #[error("Proposal inputs are empty")]
     ProposalInputsEmpty,
 
+    #[error("Proposal inputs are not unique")]
+    ProposalInputsReuse,
+
     #[error("Invalid input Merkle root")]
     InvalidInputMerkleRoot,
 
@@ -102,28 +105,29 @@ impl From<DaoError> for ContractError {
             DaoError::InvalidCalls => Self::Custom(1),
             DaoError::DaoAlreadyExists => Self::Custom(2),
             DaoError::ProposalInputsEmpty => Self::Custom(3),
-            DaoError::InvalidInputMerkleRoot => Self::Custom(4),
-            DaoError::NonMatchingSnapshotRoots => Self::Custom(5),
-            DaoError::SnapshotTooOld => Self::Custom(6),
-            DaoError::SnapshotDeserializationError => Self::Custom(7),
-            DaoError::InvalidDaoMerkleRoot => Self::Custom(8),
-            DaoError::ProposalAlreadyExists => Self::Custom(9),
-            DaoError::VoteInputsEmpty => Self::Custom(10),
-            DaoError::ProposalNonexistent => Self::Custom(11),
-            DaoError::ProposalEnded => Self::Custom(12),
-            DaoError::CoinAlreadySpent => Self::Custom(13),
-            DaoError::DoubleVote => Self::Custom(14),
-            DaoError::ExecCallWrongChildCallsLen => Self::Custom(15),
-            DaoError::ExecCallWrongChildCall => Self::Custom(16),
-            DaoError::ExecCallInvalidFormat => Self::Custom(17),
-            DaoError::ExecCallValueMismatch => Self::Custom(18),
-            DaoError::VoteCommitMismatch => Self::Custom(19),
-            DaoError::AuthXferSiblingWrongContractId => Self::Custom(20),
-            DaoError::AuthXferSiblingWrongFunctionCode => Self::Custom(21),
-            DaoError::AuthXferNonMatchingEncInputUserData => Self::Custom(22),
-            DaoError::AuthXferCallNotFoundInParent => Self::Custom(23),
-            DaoError::AuthXferWrongNumberOutputs => Self::Custom(24),
-            DaoError::AuthXferWrongOutputCoin => Self::Custom(25),
+            DaoError::ProposalInputsReuse => Self::Custom(4),
+            DaoError::InvalidInputMerkleRoot => Self::Custom(5),
+            DaoError::NonMatchingSnapshotRoots => Self::Custom(6),
+            DaoError::SnapshotTooOld => Self::Custom(7),
+            DaoError::SnapshotDeserializationError => Self::Custom(8),
+            DaoError::InvalidDaoMerkleRoot => Self::Custom(9),
+            DaoError::ProposalAlreadyExists => Self::Custom(10),
+            DaoError::VoteInputsEmpty => Self::Custom(11),
+            DaoError::ProposalNonexistent => Self::Custom(12),
+            DaoError::ProposalEnded => Self::Custom(13),
+            DaoError::CoinAlreadySpent => Self::Custom(14),
+            DaoError::DoubleVote => Self::Custom(15),
+            DaoError::ExecCallWrongChildCallsLen => Self::Custom(16),
+            DaoError::ExecCallWrongChildCall => Self::Custom(17),
+            DaoError::ExecCallInvalidFormat => Self::Custom(18),
+            DaoError::ExecCallValueMismatch => Self::Custom(19),
+            DaoError::VoteCommitMismatch => Self::Custom(20),
+            DaoError::AuthXferSiblingWrongContractId => Self::Custom(21),
+            DaoError::AuthXferSiblingWrongFunctionCode => Self::Custom(22),
+            DaoError::AuthXferNonMatchingEncInputUserData => Self::Custom(23),
+            DaoError::AuthXferCallNotFoundInParent => Self::Custom(24),
+            DaoError::AuthXferWrongNumberOutputs => Self::Custom(25),
+            DaoError::AuthXferWrongOutputCoin => Self::Custom(26),
         }
     }
 }

src/contract/dao/src/model.rs

@@ -285,6 +285,8 @@ pub struct DaoProposeParamsInput {
     pub merkle_coin_root: MerkleNode,
     /// SMT root for the input's nullifier exclusion proof
     pub smt_null_root: pallas::Base,
+    /// Input proposal specific nullifier
+    pub input_nullifier: Nullifier,
     /// Public key used for signing
     pub signature_public: PublicKey,
 }

src/contract/test-harness/src/vks.rs

@@ -49,8 +49,8 @@ use tracing::debug;
 
 /// Update these if any circuits are changed.
 /// Delete the existing cachefiles, and enable debug logging, you will see the new hashes.
-const PKS_HASH: &str = "35ce1debf6ab12d1ec6db2b8c0c2a8a9b1fd25c2ff15c1258548923ce00f781f";
-const VKS_HASH: &str = "415cb6ae64917b4dac078ac47d49408549799b71a39603d5fa4d3e6934eeece9";
+const PKS_HASH: &str = "f618ddf6916fb043200fdc1611b9a914e08d20a17e97c5a262dde874c3212fd8";
+const VKS_HASH: &str = "e083ec986b85ec3435a880363f266b793708b5fe47714a180ca548273891c1e1";
 
 /// Build a `PathBuf` to a cachefile
 fn cache_path(typ: &str) -> Result<PathBuf> {