DAO Governance Tokens Can Be Reused To Meet The Proposal Threshold
Description. The DAO contract allows the minting of new DAOs with a given threshold of governance tokens to be met for creating proposals. This threshold is checked within a “main propose” proof (src/contract/dao/proof/propose-main.zk):
circuit "ProposeMain" {
# TRUNCATED...
# This is the main check
# We check that dao_proposer_limit <= total_funds
one = witness_base(1);
total_funds_1 = base_add(total_funds, one);
less_than_strict(dao_proposer_limit, total_funds_1);
Where the total_funds is a public input to the circuit, that represents the sum of a number of governance tokens owned by the creator of the proposal.
To prove that they own that many governance tokens, the creator also accompanies a new proposal with a number of “input propose” proofs. These proofs allow the contract to check that indeed, they are owned by the creator of the proposal, and they are of the correct type, and they sum up to the total_funds input.
Unfortunately, the contract does not check that all inputs passed are distinct. As one can see in src/contract/dao/src/entrypoint/propose.rs the logic only checks that each input was not already spent:
pub(crate) fn dao_propose_process_instruction(
cid: ContractId,
call_idx: u32,
calls: Vec<DarkLeaf<ContractCall>>,
) -> Result<Vec<u8>, ContractError> {
// TRUNCATED...
for input in ¶ms.inputs {
// TRUNCATED...
// Check the coins weren't already spent
if db_contains_key(money_nullifier_db, &serialize(&input.nullifier))? {
msg!("[Dao::Vote] Error: Coin is already spent");
return Err(DaoError::CoinAlreadySpent.into())
}
}
This means that a user that does not have enough governance token to create a proposal, could simply reuse the same governance token multiple times to meet the threshold.
Recommendation. Checking for duplicate inputs requires checking for duplicate nullifiers. The same logic exists in vote.rs and could be replicated:
pub(crate) fn dao_vote_process_instruction(
cid: ContractId,
call_idx: u32,
calls: Vec<DarkLeaf<ContractCall>>,
) -> Result<Vec<u8>, ContractError> {
// TRUNCATED...
let mut vote_nullifiers = vec![];
for input in ¶ms.inputs {
// TRUNCATED...
if vote_nullifiers.contains(&input.nullifier) ||
db_contains_key(dao_vote_nullifier_db, &null_key)?
{
msg!("[Dao::Vote] Error: Attempted double vote");
return Err(DaoError::DoubleVote.into())
}
// TRUNCATED...
vote_nullifiers.push(input.nullifier);
}