Creation of a confirmation proposal can be blocked

[c4-bot-10]

Lines of code

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

Vulnerability details

Impact

The creation of a confirmation proposal can be blocked because of a front-running attack. The setting of the contract address and updating of the website can be blocked forever.

Attack scenario:

1. A proposal for setting the contract address is created:ballotName = setContract:priceFeed1 where contractName = priceFeed1
2. Users vote to approve the proposal and the ballotMinimumEndTime is passed.
3. Someone sends a transaction to the DAO contract to try to call the finalizeBallot function.
4. A malicious user front-runs the transaction and creates a new proposal with the name ballotName = setContract:priceFeed1_confirm where contractName = priceFeed1_confirm. Here,openBallotsByName[ballotName] is equal to the new ballotID.
5. After that, the correct transaction is executed and the createConfirmationProposal function is reached where the _possiblyCreateProposal function is called. The require statement for openBallotsByName[ballotName] == 0 will not be passed.

// Make sure that a proposal of the same name is not already open for the ballot
require(
     openBallotsByName[ballotName] == 0,
     "Cannot create a proposal similar to a ballot that is still open"
);

The ballotName here would be equal to setContract:priceFeed1_confirm because the name of confirmation proposal is equal to string.concat(ballot.ballotName, "_confirm")

 else if (ballot.ballotType == BallotType.SET_CONTRACT)
            proposals.createConfirmationProposal(
                string.concat(ballot.ballotName, "_confirm"), // confirmation proposal name
                BallotType.CONFIRM_SET_CONTRACT,
                ballot.address1,
                "",
                ballot.description
            );

As a result, the transaction will revert and can not create a confirmation proposal.

The malicious user can do that endlessly and block the creation of confirmation proposals forever. The only requirement is to have enough staked XSalt for creating a normal proposal.

Tools Used

Manual Review

Recommended Mitigation Steps

ReserveopenBallotsByName[ballotName] == 1, where ballotName = string.concat(ballot.ballotName, "_confirm"), during the creation of the proposal.

Assessed type

DoS

[c4-judge]

Picodes marked the issue as duplicate of #620

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by Picodes

[J4X-98]

Hey @Picodes,

thanks for reviewing the submission. I would like to add, that this is a duplicate of #980. Both exploit the exactly same issue of appending "setContract:pricefeed1 + _confirm" to the contract name and block other proposals that way.

[othernet-global]

confirm_ is now prepended to automatic confirmation ballots form setWebsiteURL and setContract proposals.

https://github.com/othernet-global/salty-io/commit/5aa1bc1ddadd67cd875de932633948af25ff8957

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #620