code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

voting power/quorum can be maniplated. #815

Closed c4-bot-7 closed 9 months ago

c4-bot-7 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L98 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L276 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L320 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L266-L269 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L89 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/staking/StakingRewards.sol#L122

Vulnerability details

Impact

  1. While voting for the ballot in Proposals.castVote, the functions use shares staked in PoolUtils.STAKED_SALT pool as quotas, those shares represent the amount of SALT staked in PoolUtils.STAKED_SALT pool. The issue is that a malicious user can call Proposals.castVote to vote, and then calls Staking.unstake to retrieve SALT back(even there might some SALT lost becaues of expeditedUnstakeFee), and transfers the remaining SALT to another wallet, and restake again, by doing this, the second wallet can also calls Proposals.castVote to vote.

  2. By using simliar method, Proposals._possiblyCreateProposal can use this methods to bypass the limitation one user can only have one active proposal

  3. And similar to Proposals.castVote, Proposals.requiredQuorumForBallotType can also be manipulated. For example, a whale who owns a large amount of SALT can first Staking.stakeSALT into PoolUtils.STAKED_SALT, and calls Proposals.castVote to vote, then, calls Staking.unstake to withdraw his SALT back. After Staking.unstake, the staking.totalShares( PoolUtils.STAKED_SALT) will be a smaller number, and require less quorum to finalized the ballot.

Proof of Concept

  1. staking.userShareForPool is used in Proposals._possiblyCreateProposal and Proposals.castVote, which is a spot value and defined as:

    // Get the user's shares for a specified pool.
    function userShareForPool( address wallet, bytes32 poolID ) public view returns (uint256)
        {
        return _userShareInfo[wallet][poolID].userShare;
        }
  2. staking.totalShares is used in Proposals.requiredQuorumForBallotType,which is also a spot value

Tools Used

VS

Recommended Mitigation Steps

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #46

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory