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

11 stars 6 forks source link

CreateProposal can be DoS #683

Closed c4-bot-2 closed 9 months ago

c4-bot-2 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/dao/Proposals.sol#L102

Vulnerability details

Impact

The attacker uses front-running to prevent the specified Proposal from being created.

Proof of Concept

    function _possiblyCreateProposal( string memory ballotName, BallotType ballotType, address address1, uint256 number1, string memory string1, string memory string2 ) internal returns (uint256 ballotID)
    {
        ......
        require( openBallotsByName[ballotName] == 0, "Cannot create a proposal similar to a ballot that is still open" );
        .....
    }

The Proposal is created by checking whether ballotName already exists in openBallotsByName, and if it does, it cannot be created.

The problem is that if an attacker creates a Proposal with this ballotName in advance, the Proposal cannot be created.

openBallotsByName[ballotName], is not removed until the vote is complete, so a Proposal created by an attacker will never be removed if no one votes on it.

    function markBallotAsFinalized( uint256 ballotID ) external nonReentrant{
        ......
        delete openBallotsByName[ballot.ballotName];
        ......
    }

For example, proposeParameterBallot, anything can be called:

    function proposeParameterBallot( uint256 parameterType, string calldata description ) external nonReentrant returns (uint256 ballotID)
    {
        string memory ballotName = string.concat("parameter:", Strings.toString(parameterType) );
        return _possiblyCreateProposal( ballotName, BallotType.PARAMETER, address(0), parameterType, "", description );
    }

The attacker finds that there is a proposeParameterBallot to be executed. The attacker executes the same function, uses the same parameterType parameter, keeps ballotName consistent, uses any description, and passesfront-running Make the transaction execute in advance so that the compromised proposeParameterBallot cannot be successfully created.

Tools Used

vscode, manual

Recommended Mitigation Steps

hash the other parameters of the Ballot to determine the uniqueness of the Ballot, not by name.

Assessed type

DoS

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #621

c4-judge commented 8 months ago

Picodes marked the issue as satisfactory