User can call `StandardFunding.updateSlate` function to frontrun other user's `StandardFunding.updateSlate` transaction

[code423n4]

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L300-L340 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L421-L454 https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L488-L497

Vulnerability details

Impact

During the challenge phase, the following StandardFunding.updateSlate function can be called, which further calls the StandardFunding._validateSlate function below. Since the StandardFunding._validateSlate function executes if (totalTokensRequested > (gbc * 9 / 10)) {revert InvalidProposalSlate();}, the total number of tokens requested by all proposals to be added to _fundedProposalSlates[newSlateHash] cannot exceed gbc * 9 / 10. Moreover, because the StandardFunding.updateSlate function executes newTopSlate_ = currentSlateHash == 0 || (currentSlateHash!= 0 && sum > _sumProposalFundingVotes(_fundedProposalSlates[currentSlateHash])), the sum of fundingVotesReceived for all proposals to be added to _fundedProposalSlates[newSlateHash] must be higher than that for the current slate's proposals for winning the challenge and updating the slate.

After the funding phase, it is possible that not all of the top ten proposals can be executed because their total number of requested tokens can exceed gbc * 9 / 10 and some of these top ten proposals have equal fundingVotesReceived. For example, fundingVotesReceived of the top 9th and 10th proposals can be equal, and the gbc * 9 / 10 limit on the total number of requested tokens can only allow the top 9th or 10th proposal to be added to _fundedProposalSlates[newSlateHash] along with the other eight top proposals. The proposer of the top 9th proposal can call the StandardFunding.updateSlate function for adding the other top eight proposals and the top 9th proposal to _fundedProposalSlates[newSlateHash]. After noticing this transaction of the top 9th proposal's proposer in the mempool, the proposer of the top 10th proposal or someone who does not support the top 9th proposal can frontrun such transaction by calling the StandardFunding.updateSlate function for adding the other top eight proposals and the top 10th proposal to _fundedProposalSlates[newSlateHash]. After the frontrunning, the other top eight proposals and the top 10th proposal are added to _fundedProposalSlates[newSlateHash], and then sum > _sumProposalFundingVotes(_fundedProposalSlates[currentSlateHash]) and newTopSlate_ would become false when executing the transaction of the top 9th proposal's proposer. Because newTopSlate_ is false, the top 9th proposal cannot be added to _fundedProposalSlates[newSlateHash] and thus cannot be executed even though it should be if such frontrunning can be prevented, which is unfair to the top 9th proposal.

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L300-L340

    function updateSlate(
        uint256[] calldata proposalIds_,
        uint24 distributionId_
    ) external override returns (bool newTopSlate_) {
        QuarterlyDistribution storage currentDistribution = _distributions[distributionId_];

        // store number of proposals for reduced gas cost of iterations
        uint256 numProposalsInSlate = proposalIds_.length;

        // check the each proposal in the slate is valid, and get the sum of the proposals fundingVotesReceived
        uint256 sum = _validateSlate(distributionId_, currentDistribution.endBlock, currentDistribution.fundsAvailable, proposalIds_, numProposalsInSlate);

        // get pointers for comparing proposal slates
        bytes32 currentSlateHash = currentDistribution.fundedSlateHash;
        bytes32 newSlateHash     = keccak256(abi.encode(proposalIds_));

        // check if slate of proposals is better than the existing slate, and is thus the new top slate
        newTopSlate_ = currentSlateHash == 0 ||
            (currentSlateHash!= 0 && sum > _sumProposalFundingVotes(_fundedProposalSlates[currentSlateHash]));

        // if slate of proposals is new top slate, update state
        if (newTopSlate_) {
            uint256[] storage existingSlate = _fundedProposalSlates[newSlateHash];

            for (uint i = 0; i < numProposalsInSlate; ) {

                // update list of proposals to fund
                existingSlate.push(proposalIds_[i]);

                unchecked { ++i; }
            }

            // update hash to point to the new leading slate of proposals
            currentDistribution.fundedSlateHash = newSlateHash;

            ...
        }
    }

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

    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; }
        }
    }

https://github.com/code-423n4/2023-05-ajna/blob/main/ajna-grants/src/grants/base/StandardFunding.sol#L488-L497

    function _sumProposalFundingVotes(
        uint256[] memory proposalIdSubset_
    ) internal view returns (uint128 sum_) {
        for (uint i = 0; i < proposalIdSubset_.length;) {
            // since we are converting from int128 to uint128, we can safely assume that the value will not overflow
            sum_ += uint128(_standardFundingProposals[proposalIdSubset_[i]].fundingVotesReceived);

            unchecked { ++i; }
        }
    }

Proof of Concept

The following steps can occur for the described scenario.

1. After the funding phase, Alice and Bob's proposals are two of the top ten proposals but received the lowest and equal fundingVotesReceived. The gbc * 9 / 10 limit on the total number of requested tokens only allows the other top eight proposals and Alice or Bob's proposal to be added to _fundedProposalSlates[newSlateHash].
2. Alice calls the StandardFunding.updateSlate function for adding the other top eight proposals and her proposal to _fundedProposalSlates[newSlateHash].
3. After noticing Alice's transaction, Bob frontruns Alice's transaction by also calling the StandardFunding.updateSlate function for adding the other top eight proposals and his proposal to _fundedProposalSlates[newSlateHash].
4. After the frontrunning, Bob's proposal is added to _fundedProposalSlates[newSlateHash] and can be executed after the challenge phase is ended but Alice's proposal cannot be added to _fundedProposalSlates[newSlateHash] and cannot be executed after the challenge phase is over, which is unexpected and unfair to Alice.

Tools Used

VSCode

Recommended Mitigation Steps

A tie breaker can be added to determine the priorities among the proposals that have equal fundingVotesReceived after the funding phase is ended. Such tie breaker can be the time when the proposals were created, the number of tokens requested by the proposals, etc. or a combination of these factors. The StandardFunding.updateSlate function can then be updated to allow the proposal with the higher priority to replace the proposal with the lower priority in _fundedProposalSlates[newSlateHash] when both proposals have equal fundingVotesReceived.

Assessed type

DoS

[Picodes]

Seems Low severity to me as it is an edge case and the suggested tie breakers are arbitrary as well

[c4-judge]

Picodes marked the issue as primary issue

[Picodes]

Related to https://github.com/code-423n4/2023-05-ajna-findings/issues/209

[c4-judge]

Picodes marked issue #129 as primary and marked this issue as a duplicate of 129

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)