code-423n4 / 2023-05-ajna-findings

2 stars 0 forks source link

Proposals with no votes may still be capable of receiving funding. #372

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-grants/src/grants/base/StandardFunding.sol#L421-L454

Vulnerability details

Impact

Proposals with no votes may still be capable of receiving funding, leading to unexpected protocol behaviour.

Proof of Concept

After the voting period for proposals ends, there is a one-week challenge period during which anyone can submit a new set of winning proposals. The winning proposals are then decided based on which set is more optimal than the previous set of winning proposals. This process is facilitated by a function called updateSlate, which can be found at a specific location in the code. To ensure the validity of a potential slate of proposals, another function called _validateSlate is called, which also calculates the total amount of funding votes received by the slate.

Let's examine: _validateState()

    function _validateSlate(uint24 distributionId_, uint256 endBlock, uint256 distributionPeriodFundsAvailable_, uint256[] calldata proposalIds_, uint256 numProposalsInSlate_) internal view returns (uint256 sum_) {
        // check that the function is being called within the challenge period
        if (block.number <= endBlock || block.number > _getChallengeStageEndBlock(endBlock)) {
            revert InvalidProposalSlate();
        }

        // check that the slate has no duplicates
        if (_hasDuplicates(proposalIds_)) revert InvalidProposalSlate();

        uint256 gbc = distributionPeriodFundsAvailable_;
        uint256 totalTokensRequested = 0;

        // check each proposal in the slate is valid
        for (uint i = 0; i < numProposalsInSlate_; ) {
            Proposal memory proposal = _standardFundingProposals[proposalIds_[i]];

            // check if Proposal is in the topTenProposals list
            if (_findProposalIndex(proposalIds_[i], _topTenProposals[distributionId_]) == -1) revert InvalidProposalSlate();

            // account for fundingVotesReceived possibly being negative
            if (proposal.fundingVotesReceived < 0) revert InvalidProposalSlate();

            // update counters
            sum_ += uint128(proposal.fundingVotesReceived); // since we are converting from int128 to uint128, we can safely assume that the value will not overflow
            totalTokensRequested += proposal.tokensRequested;

            // check if slate of proposals exceeded budget constraint ( 90% of GBC )
            if (totalTokensRequested > (gbc * 9 / 10)) {
                revert InvalidProposalSlate();
            }

            unchecked { ++i; }
        }
    }

If the proposals in the submitted slate are part of the top ten proposals list and none of the proposals received negative funding votes, then the total funding votes received by all proposals in the slate will be returned. However, even if a proposal received zero votes, it can still be eligible for funding if there is no other proposal with a higher number of votes. This lack of check for zero funding votes is confirmed by the sponsor team and can be observed in a specific line of code.

            if (proposal.fundingVotesReceived < 0) revert InvalidProposalSlate();

Tools Used

Manual Review

Recommended Mitigation Steps

Consider implementing the following change to include proposals with 0 votes

            if (proposal.fundingVotesReceived <= 0) revert InvalidProposalSlate();

Assessed type

Invalid Validation

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #465

c4-judge commented 1 year ago

Picodes marked the issue as partial-25

Picodes commented 1 year ago

Partial duplicate of https://github.com/code-423n4/2023-05-ajna-findings/issues/465 missing the main attack path and issue.