c4-bot-2 commented 5 months ago

Lines of code

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

Vulnerability details

Proof of Concept

Steps to generate the issue

First the Dao creates 5 proposals through _possiblyCreateProposal, daos are allowed to create more than one proposal
Call markBallotAsFinalized(ballotId) where ballotId is an id of the five created proposals by the dao
Call _userHasActiveProposal[daoAddress], return value is false while the dao has another 4 open proposals

The issue arises because the dao is allowed to create multiple proposals, while a user cannot, in the markBallotAsFinalized function implementation, only the case of a user creating only one proposal and deleting it is considered, while the case that the dao might have more than one proposal is not considered. Proposals.sol #145-147


    function markBallotAsFinalized( uint256 ballotID ) external nonReentrant
        {
            ...
145     // Indicate that the user who posted the proposal no longer has an active proposal
146     address userThatPostedBallot = _usersThatProposedBallots[ballotID];
147     _userHasActiveProposal[userThatPostedBallot] = false;
            ...
        }

In the following discussion with the sponsor, he acknowledged the issue, by confirming that daos are allowed to create more than one proposal, and from the code, we can see clearly that the case is not handled

chat

Tools Used

Manual Review, vs code

Recommended Mitigation Steps

Have separate logic proposal deletion in case of a dao is the one created the proposal, check if the dao has more proposal and if none then remove.

Assessed type

Governance

c4-judge commented 5 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 5 months ago

othernet-global (sponsor) disputed

othernet-global commented 5 months ago

_userHasActiveProposal is not applicable to the DAO and the behavior is acceptable.