Proposals that didn't reach quorum should be able to be finalized without changes when the voting phase ends

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/DAO.sol#L280-L281
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L384-L385
https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L395-L397

Vulnerability details

Impact

The current voting system specifies a minimum end time and forces each ballot to reach quorum before the proposal can be finalized. It also allows voting on the proposal after the minimal end time, with the same voting power as if done before that.

This creates a security risk, as malicious proposals may remain alive indefinitely after the minimal voting period ends, as long as there is no quorum.

It is important to note that the minimum quorum can reach values up to 60% of staked assets, which will make it hard to reach for every ballot at any moment in time. (60% calculated as 3X the max base ballot quorum percent).

Also, not only for malicious proposals, but for any proposal, the users that see that their option is losing can withdraw their votes, so that there is not enough quorum to finalize it. This way they can postpone the decision. They can also silently accumulate voting power, and flip the election later, filling the quorum gap.

Another point is that new proposals can't be created with the same ballot name. In some cases, that prevents creating some specific type of proposal altogether, like the "Send SALT" proposal. So, if a not so popular proposal doesn't reach quorum, it will block others, not being able to remove it, even when the minimum voting period ended.

Note: This Impact fits into the Attack Ideas: "Any issue that would prevent the DAO from functioning correctly."

Vulnerability Details

When trying to finalize a ballot, there is a check after evaluating if the minimum end time was reached, that asserts quorum was reached. If it returns false, the tx will revert.

// Check that the required quorum has been reached
if ( totalVotesCastForBallot(ballotID) < requiredQuorumForBallotType( ballot.ballotType ))
    return false;

Proposals.sol#L395-L397

This can be exploited as explained on the Impact section. Users can for example, remove their votes just before the minimum end time, to prevent reaching quorum, and add their voting power later, when they see they can reach quorum and flip the election.

In the meantime, proposals with the same ballot name can't be created, like sending SALT, or changing the address of a specific price feed for example.

Proof of Concept

1. Add this test to src/dao/tests/DAO.t.sol
2. Run COVERAGE="no" NETWORK="sep" forge test -vv --rpc-url https://1rpc.io/sepolia --mt "testManipulatingQuorum"

function testManipulatingQuorum() public {
    // Alice creates a proposal
    vm.startPrank(alice);
    staking.stakeSALT(1000000 ether);
    address newAddress = address(0x1111111111111111111111111111111111111112);
    uint256 ballotID = proposals.proposeSetContractAddress("priceFeed1_confirm", newAddress, "description");

    // She casts her vote, at the same time increasing the votes needed to reach the quorum
    proposals.castVote(ballotID, Vote.YES);
    assertEq(proposals.totalVotesCastForBallot(ballotID), 1000000 ether);

    // The time passes and the ballot reaches its minimum end duration
    vm.warp(block.timestamp + daoConfig.ballotMinimumDuration());

    // Alice realizes her option won't win the ballot
    // She can manipulate the quorum, by "withdrawing" her votes
    uint256 unstakeID = staking.unstake(1000000 ether - 100, 52);
    proposals.castVote(ballotID, Vote.YES);
    staking.cancelUnstake(unstakeID);

    // Now the quorum is reduced
    assertEq(proposals.totalVotesCastForBallot(ballotID), 100);

    // And the ballot can't be finalized
    vm.expectRevert("The ballot is not yet able to be finalized");
    dao.finalizeBallot(ballotID);

    // At any point in time in the future Alice can cast her vote again if she foresees that she can win
    // She may even stake more tokens to reach quorum + win the ballot
    staking.stakeSALT(1000000 ether);
    proposals.castVote(ballotID, Vote.YES);
    assertEq(proposals.totalVotesCastForBallot(ballotID), 2000000 ether);

    // And immediately finalize the ballot
    dao.finalizeBallot(ballotID);
}

Recommended Mitigation Steps

Allow ballots to be finalized without changes if they reach the minimum end time without quorum. Verify that any assumptions about quorum are not broken like in this case that assumes that "uorum would already have been determined".

Assessed type

Governance

[c4-judge]

Picodes marked the issue as duplicate of #362

[c4-judge]

Picodes marked the issue as satisfactory