Treasury accounting miss voters rewards

[code423n4]

Lines of code

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

Vulnerability details

treasury is overstated over time as each distribution period it adds back the delegate rewards part, which is actually spent on voters rewards.

I.e. it is updated with fundsAvailable - totalTokensRequested difference, while totalTokensRequested is limited to 90% of the fundsAvailable. There is also 10% of the cumulative voter reward part, which can be requested by the voters anytime (amounts are fixed and there is no upper time limit for requesting).

This way each standard proposal will overstate treasury by 10% of the current period budget. I.e. as totalTokensRequested has '90% fundsAvailable' as a maximum , the fundsAvailable - totalTokensRequested will always be at least `10% fundsAvailable. This part, however, is fully spent on delegators rewards and is not available to be added back to the future periods' unallocated budgets, i.e. totreasury`.

Impact

Delegation reward spending is not accounted this way in the treasury funds tracking and so treasury available funds are overstated more and more along with each new distribution period start.

It means that GLOBAL_BUDGET_CONSTRAINT is generally violated and eventually it will lead to inability to fund successful proposals (insolvency) as the controlling checks will be too loose, not being aligned with the actual funds available.

Proof of Concept

treasury is updated with fundsAvailable - totalTokensRequested difference, i.e. what is requested minus what is to be transferred away:

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

    function _updateTreasury(
        uint24 distributionId_
    ) private {
        bytes32 fundedSlateHash = _distributions[distributionId_].fundedSlateHash;
        uint256 fundsAvailable  = _distributions[distributionId_].fundsAvailable;

        uint256[] memory fundingProposalIds = _fundedProposalSlates[fundedSlateHash];

        uint256 totalTokensRequested;
        uint256 numFundedProposals = fundingProposalIds.length;

        for (uint i = 0; i < numFundedProposals; ) {
            Proposal memory proposal = _standardFundingProposals[fundingProposalIds[i]];

            totalTokensRequested += proposal.tokensRequested;

            unchecked { ++i; }
        }

        // readd non distributed tokens to the treasury
>>      treasury += (fundsAvailable - totalTokensRequested);

        _isSurplusFundsUpdated[distributionId_] = true;
    }

But totalTokensRequested is limited to '90%' of the funds available in the distribution period:

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

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

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

        uint256 gbc                           = Maths.wmul(treasury, GLOBAL_BUDGET_CONSTRAINT);
        newDistributionPeriod.fundsAvailable  = SafeCast.toUint128(gbc);

The other 10% is allocated to be grabbed by the voters, i.e. 10% of the currentDistribution_.fundsAvailable is distributed as voters rewards:

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

    function claimDelegateReward(
        uint24 distributionId_
    ) external override returns(uint256 rewardClaimed_) {
        // Revert if delegatee didn't vote in screening stage
        if(screeningVotesCast[distributionId_][msg.sender] == 0) revert DelegateRewardInvalid();

        QuarterlyDistribution memory currentDistribution = _distributions[distributionId_];

        // Check if Challenge Period is still active
        if(block.number < _getChallengeStageEndBlock(currentDistribution.endBlock)) revert ChallengePeriodNotEnded();

        // check rewards haven't already been claimed
        if(hasClaimedReward[distributionId_][msg.sender]) revert RewardAlreadyClaimed();

        QuadraticVoter memory voter = _quadraticVoters[distributionId_][msg.sender];

        // calculate rewards earned for voting
>>      rewardClaimed_ = _getDelegateReward(currentDistribution, voter);

        hasClaimedReward[distributionId_][msg.sender] = true;

        emit DelegateRewardClaimed(
            msg.sender,
            distributionId_,
            rewardClaimed_
        );

        // transfer rewards to delegatee
>>      IERC20(ajnaTokenAddress).safeTransfer(msg.sender, rewardClaimed_);
    }

votingPowerAllocatedByDelegatee = voter_.votingPower - voter_.remainingVotingPower is a cumulative voting done by the voter, while currentDistribution_.fundingVotePowerCast is the sum of those:

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

    function _getDelegateReward(
        QuarterlyDistribution memory currentDistribution_,
        QuadraticVoter memory voter_
    ) internal pure returns (uint256 rewards_) {
        // calculate the total voting power available to the voter that was allocated in the funding stage
        uint256 votingPowerAllocatedByDelegatee = voter_.votingPower - voter_.remainingVotingPower;

        // if none of the voter's voting power was allocated, they receive no rewards
        if (votingPowerAllocatedByDelegatee == 0) return 0;

        // calculate reward
        // delegateeReward = 10 % of GBC distributed as per delegatee Voting power allocated
        rewards_ = Maths.wdiv(
            Maths.wmul(
>>              currentDistribution_.fundsAvailable,
>>              votingPowerAllocatedByDelegatee
            ),
>>          currentDistribution_.fundingVotePowerCast
        ) / 10;
    }

This way votingPowerAllocatedByDelegatee sums up across all the voters to be currentDistribution_.fundingVotePowerCast and it is full currentDistribution_.fundsAvailable / 10 to be distributed to the voters.

Recommended Mitigation Steps

As rewards can be claimed anytime in the future, the whole 10% to be allocated to that within total available amount accounting, i.e. the update can look like:

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

    function _updateTreasury(
        uint24 distributionId_
    ) private {
        bytes32 fundedSlateHash = _distributions[distributionId_].fundedSlateHash;
        uint256 fundsAvailable  = _distributions[distributionId_].fundsAvailable;

        uint256[] memory fundingProposalIds = _fundedProposalSlates[fundedSlateHash];

        uint256 totalTokensRequested;
        uint256 numFundedProposals = fundingProposalIds.length;

        for (uint i = 0; i < numFundedProposals; ) {
            Proposal memory proposal = _standardFundingProposals[fundingProposalIds[i]];

            totalTokensRequested += proposal.tokensRequested;

            unchecked { ++i; }
        }

        // readd non distributed tokens to the treasury
-       treasury += (fundsAvailable - totalTokensRequested);
+       treasury += (fundsAvailable * 9 / 10 - totalTokensRequested);

        _isSurplusFundsUpdated[distributionId_] = true;
    }

Assessed type

Governance

[c4-judge]

Picodes marked the issue as duplicate of #263

[c4-judge]

Picodes marked the issue as duplicate of #263

[c4-judge]

Picodes marked the issue as satisfactory