Proposals can be generated with less than the intended ProposalPercentage

[c4-bot-7]

Lines of code

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

Vulnerability details

Bug Description

In the Salty protocol's DAO, to create proposals, it's required that proposers have a stake in the protocol, governed by the requiredProposalPercentStakeTimes1000 parameter. However, the _possiblyCreateProposal() function incorrectly rounds the calculation to determine if a user meets this staking requirement. This rounding error allows users with slightly less than the required stake percentage to still propose ballots.

uint256 totalStaked = staking.totalShares(PoolUtils.STAKED_SALT);
uint256 requiredXSalt = ( totalStaked * daoConfig.requiredProposalPercentStakeTimes1000() ) / ( 100 * 1000 );

require( requiredXSalt > 0, "requiredXSalt cannot be zero" );

uint256 userXSalt = staking.userShareForPool( msg.sender, PoolUtils.STAKED_SALT );
require( userXSalt >= requiredXSalt, "Sender does not have enough xSALT to make the proposal" );

The issue arises from the calculation of requiredXSalt, which is rounded down, setting the actual required staked salt amount lower than the intended threshold:

Impact

This problem enables users who have staked marginally less than the ProposalPercentStake to propose proposals. Although the deviation caused by rounding is minor, the principle of the ProposalPercentStake should be strictly adhered to, ensuring that no undercuts occur, even by minor values

Proof of Concept

A simplified example demonstrates the rounding issue:

// requiredXSalt = ( totalStaked * daoConfig.requiredProposalPercentStakeTimes1000() ) / ( 100 * 1000 );

rS = (199 * (2 * 1000)) / (100 * 1000)
rS = 199 / 50 = 3 // Which is rounded down

In this example, with 199 total staked SALT and a requirement of 2% to generate a proposal, the exact requirement would be 3.98 SALT. Due to Solidity's rounding down, requiredXSalt would incorrectly be calculated as 3, allowing users with only 3 SALT to propose, which is below the intended threshold.

Tools Used

Manual Review

Recommended Mitigation Steps

To address this, the calculation for requiredXSalt should round up, ensuring that the actual staked amount aligns with or exceeds the intended threshold. This can be achieved using OpenZeppelin's Math.ceilDiv() function:

uint256 requiredXSalt = Math.ceilDiv(totalStaked * daoConfig.requiredProposalPercentStakeTimes1000(), 100 * 1000);

Implementing this change will ensure that the staking requirement for proposing ballots in the DAO accurately reflects the protocol's intention, upholding the integrity of the proposal process.

Assessed type

Math

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid

[J4X-98]

Hey @Picodes,

thanks for reviewing the submission. I would like to add that here one of the intended invariants requiredXSalt >= totalStaked * proposalPercentage is broken. Additionally, I would like to add that this issue can easily be mitigated by the described fix. Also, this is definitely a valid issue, so if you can't agree on the medium severity, it can be downgraded to QA and considered for my QA report.

[c4-judge]

Picodes removed the grade

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[Picodes]

@J4X-98 I've downgraded this to QA. I don't consider broken invariants to be of Medium severity if they have no impact on the safety of users and the functionality of the protocol.