Ajna Protocol - hyh's results

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

Vulnerability details

positionIndex.remove(params_.fromIndex)removes the PositionManager entry even when it is only partial removal as a result of IPool(params_.pool).moveQuoteToken(...) call.

I.e. it is correct to do fromPosition.lps -= vars.lpbAmountFrom, but the resulting amount might not be zero, moveQuoteToken() are not guaranteed to clear the position as it has available liquidity constraint. In the case of partial quote funds removal positionIndex.remove(params_.fromIndex) operation will freeze the remaining position.

Impact

Permanent fund freeze for the remaining position of LP beneficiary.

Proof of Concept

While positions[params_.tokenId][params_.fromIndex] LP shares are correctly reduced by the amount returned by pool's moveQuoteToken(), the position itself is unconditionally removed from the positionIndexes[params_.tokenId], making any remaining funds unavailable:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

        EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

        // remove bucket index from which liquidity is moved from tracked positions
>>      if (!positionIndex.remove(params_.fromIndex)) revert RemovePositionFailed();

        // update bucket set at which a position has liquidity
        // slither-disable-next-line unused-return
        positionIndex.add(params_.toIndex);

        // move quote tokens in pool
        (
            vars.lpbAmountFrom,
            vars.lpbAmountTo,
        ) = IPool(params_.pool).moveQuoteToken(
            vars.maxQuote,
            params_.fromIndex,
            params_.toIndex,
            params_.expiry
        );

        Position storage toPosition = positions[params_.tokenId][params_.toIndex];

        // update position LP state
>>      fromPosition.lps -= vars.lpbAmountFrom;
        toPosition.lps   += vars.lpbAmountTo;
        // update position deposit time to the from bucket deposit time
        toPosition.depositTime = vars.depositTime;

Bucket can contain a mix of quote and collateral tokens, but moveLiquidity() aims to retrieve vars.maxQuote = _lpToQuoteToken(...) quote funds per current exchange rate:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/libraries/helpers/PoolHelper.sol#L222-L236

    function _lpToQuoteToken(
        uint256 bucketLP_,
        uint256 bucketCollateral_,
        uint256 deposit_,
        uint256 lenderLPBalance_,
        uint256 maxQuoteToken_,
        uint256 bucketPrice_
    ) pure returns (uint256 quoteTokenAmount_) {
        uint256 rate = Buckets.getExchangeRate(bucketCollateral_, bucketLP_, deposit_, bucketPrice_);

        quoteTokenAmount_ = Maths.wmul(lenderLPBalance_, rate);

        if (quoteTokenAmount_ > deposit_)       quoteTokenAmount_ = deposit_;
        if (quoteTokenAmount_ > maxQuoteToken_) quoteTokenAmount_ = maxQuoteToken_;
    }

There might be not enough quote deposit funds available to redeem the whole quote amount requested, which is controlled by the corresponding liquidity constraint:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/libraries/external/LenderActions.sol#L711-L719

        uint256 scaledLpConstraint = Maths.wmul(params_.lpConstraint, exchangeRate);
        if (
>>          params_.depositConstraint < scaledDepositAvailable &&
            params_.depositConstraint < scaledLpConstraint
        ) {
            // depositConstraint is binding constraint
            removedAmount_ = params_.depositConstraint;
>>          redeemedLP_    = Maths.wdiv(removedAmount_, exchangeRate);
        }

Recommended Mitigation Steps

As a most straightforward solution consider reverting when there is a remainder, i.e. when fromPosition.lps > dust_threshold:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

        EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

        // remove bucket index from which liquidity is moved from tracked positions
        if (!positionIndex.remove(params_.fromIndex)) revert RemovePositionFailed();

        // update bucket set at which a position has liquidity
        // slither-disable-next-line unused-return
        positionIndex.add(params_.toIndex);

        // move quote tokens in pool
        (
            vars.lpbAmountFrom,
            vars.lpbAmountTo,
        ) = IPool(params_.pool).moveQuoteToken(
            vars.maxQuote,
            params_.fromIndex,
            params_.toIndex,
            params_.expiry
        );

        Position storage toPosition = positions[params_.tokenId][params_.toIndex];

        // update position LP state
>>      fromPosition.lps -= vars.lpbAmountFrom;
        toPosition.lps   += vars.lpbAmountTo;
        // update position deposit time to the from bucket deposit time
        toPosition.depositTime = vars.depositTime;

Assessed type

Error

Lines of code

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

Vulnerability details

moveLiquidity() set new destination index LP entry deposit time to be equal to the source index deposit time, while destination bucket might have defaulted after that time.

This is generally not correct as source bucket bankruptcy is controlled (i.e. LP shares that are moved are healthy), while the destination bucket's bankruptcy time, being arbitrary, can be higher than source index deposit time, and in this case the funds will become inaccessible after such a move (i.e. healthy shares will be marked as defaulted due to incorrect deposit time used).

In other words the funds are moved from healthy non-default zone to an arbitrary point, which can be either healthy or not. In the latter case this constitutes a loss for an owner as toIndex bucket bankruptcy time exceeding deposit time means that all other retrieval operations will be blocked.

Impact

Owner will permanently lose access to the LP shares whenever positions[params_.tokenId][params_.toIndex] bucket bankruptcy time is greater than positions[params_.tokenId][params_.fromIndex].depositTime.

moveLiquidity() is a common operation, while source and destination bucket bankruptcy times can be related in an arbitrary manner, and the net impact is permanent fund freeze, so this is a fund loss without material prerequisites, setting the severity to be high.

Proof of Concept

moveLiquidity() sets toPosition deposit time to be fromPosition.depositTime:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
>>      vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo(params_.fromIndex);

        // check that bucket hasn't gone bankrupt since memorialization
        if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

        // calculate the max amount of quote tokens that can be moved, given the tracked LP
        vars.maxQuote = _lpToQuoteToken(
            vars.bucketLP,
            vars.bucketCollateral,
            vars.bucketDeposit,
            fromPosition.lps,
            vars.bucketDeposit,
            _priceAt(params_.fromIndex)
        );

        EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

        // remove bucket index from which liquidity is moved from tracked positions
        if (!positionIndex.remove(params_.fromIndex)) revert RemovePositionFailed();

        // update bucket set at which a position has liquidity
        // slither-disable-next-line unused-return
        positionIndex.add(params_.toIndex);

        // move quote tokens in pool
        (
            vars.lpbAmountFrom,
            vars.lpbAmountTo,
        ) = IPool(params_.pool).moveQuoteToken(
            vars.maxQuote,
            params_.fromIndex,
            params_.toIndex,
            params_.expiry
        );

        Position storage toPosition = positions[params_.tokenId][params_.toIndex];

        // update position LP state
        fromPosition.lps -= vars.lpbAmountFrom;
        toPosition.lps   += vars.lpbAmountTo;
        // update position deposit time to the from bucket deposit time
>>      toPosition.depositTime = vars.depositTime;

I.e. there is no check for params_.toIndex bucket situation, the time is just copied.

While there is checking logic in LenderActions, which checks for toBucket bankruptcy and sets the time accordingly:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/libraries/external/LenderActions.sol#L315-L327

        vars.toBucketDepositTime = toBucketLender.depositTime;
        if (vars.toBucketBankruptcyTime >= vars.toBucketDepositTime) {
            // bucket is bankrupt and deposit was done before bankruptcy time, reset lender lp amount
            toBucketLender.lps = toBucketLP_;

            // set deposit time of the lender's to bucket as bucket's last bankruptcy timestamp + 1 so deposit won't get invalidated
            vars.toBucketDepositTime = vars.toBucketBankruptcyTime + 1;
        } else {
            toBucketLender.lps += toBucketLP_;
        }

        // set deposit time to the greater of the lender's from bucket and the target bucket
        toBucketLender.depositTime = Maths.max(vars.fromBucketDepositTime, vars.toBucketDepositTime);

This way, while bucket structure deposit time will be controlled and updated, PositionManager's structure will have the deposit time copied over.

In the case when positions[params_.tokenId][params_.fromIndex].depositTime was less than params_.toIndex bankruptcyTime, this will freeze these LP funds as further attempts to use them will be blocked:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L285

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
>>      vars.depositTime = fromPosition.depositTime;

        // handle the case where owner attempts to move liquidity after they've already done so
        if (vars.depositTime == 0) revert RemovePositionFailed();

        // ensure bucketDeposit accounts for accrued interest
        IPool(params_.pool).updateInterest();

        // retrieve info of bucket from which liquidity is moved  
        (
            vars.bucketLP,
            vars.bucketCollateral,
>>          vars.bankruptcyTime,
            vars.bucketDeposit,
        ) = IPool(params_.pool).bucketInfo();

        // check that bucket hasn't gone bankrupt since memorialization
>>      if (vars.depositTime <= vars.bankruptcyTime) revert BucketBankrupt();

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L352-L372

    function reedemPositions(
        RedeemPositionsParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) {
        EnumerableSet.UintSet storage positionIndex = positionIndexes[params_.tokenId];

        ...

        for (uint256 i = 0; i < indexesLength; ) {
            index = params_.indexes[i];

            Position memory position = positions[params_.tokenId][index];

            if (position.depositTime == 0 || position.lps == 0) revert RemovePositionFailed();

            // check that bucket didn't go bankrupt after memorialization
>>          if (_bucketBankruptAfterDeposit(pool, index, position.depositTime)) revert BucketBankrupt();

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L436-L443

    function _bucketBankruptAfterDeposit(
        IPool pool_,
        uint256 index_,
        uint256 depositTime_
    ) internal view returns (bool) {
        (, , uint256 bankruptcyTime, , ) = pool_.bucketInfo(index_);
>>      return depositTime_ <= bankruptcyTime;
    }

Recommended Mitigation Steps

Consider using the resulting time of the destination position, for example:

https://github.com/code-423n4/2023-05-ajna/blob/276942bc2f97488d07b887c8edceaaab7a5c3964/ajna-core/src/PositionManager.sol#L262-L323

    function moveLiquidity(
        MoveLiquidityParams calldata params_
    ) external override mayInteract(params_.pool, params_.tokenId) nonReentrant {
        Position storage fromPosition = positions[params_.tokenId][params_.fromIndex];

        MoveLiquidityLocalVars memory vars;
        vars.depositTime = fromPosition.depositTime;

        ...

        Position storage toPosition = positions[params_.tokenId][params_.toIndex];

        // update position LP state
        fromPosition.lps -= vars.lpbAmountFrom;
        toPosition.lps   += vars.lpbAmountTo;
-       // update position deposit time to the from bucket deposit time
+       // update position deposit time with the renewed to bucket deposit time
+       (, vars.depositTime) = pool.lenderInfo(params_.toIndex, address(this));
        toPosition.depositTime = vars.depositTime;

Notice, that this time value will be influenced by the other PositionManager positions in the params_.toIndex bucket, but the surface described will be closed as it will be controlled against params_.toIndex bucket bankruptcy time.

Assessed type

Error

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. to treasury.

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

Lines of code

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

Vulnerability details

Attacker can monitor the standard proposals distribution and routinely steal each low activity period remainder by submitting a transfer to self proposal and voting a dust amount for it.

Since the criteria for the final slate update is that any increase in total funding votes casted is enough, the attacker's costs are negligible, while the remainder funds during some periods can be substantial enough for the attacker to setup such a monitoring. I.e. as funds are constant share of the treasury, while activity can differ drastically, a situation when there are less viable proposals then funds can routinely happen over time.

The assumption of the current logic is that such unallocated funds will be returned to the treasury, but it will not be the case as the cost of stealing such funds is close to zero.

Impact

A part of treasury funds can be stolen each period and will not be available for ecosystem funding.

Proof of Concept

Schematic POC:

1. Bob monitors the end of each screening period and, whenever it is cheap enough, submits a proposal to send the remainder funds to self via proposeStandard()
2. Bob votes for it with fundingVote() with the dust votes he have. Since it is low activity period there are room, and it is included to _topTenProposals
3. Bob updates the top slate with updateSlate(), repeating current top slate with his proposal added. Since other proposals cumulatively do not allocate full budget and Bob's proposal have positive funding vote attached, it is included to the slate

This way Bob obtained the remainder funds nearly for free.

Core issue here looks to be the absence of the proposal votes threshold, which allows an attacker to claim the remained without any barrier to entry, i.e. having at hand only dust amount of governance tokens.

Even proposal with zero funding votes can be executed, it is only controlled to be non-negative:

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

    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();

The only criteria for state update is greater sum of the funding votes:

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

        // 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]));

I.e. when the activity is low enough attacker can always maximize the totalTokensRequested to be exactly gbc * 9 / 10, claiming the difference to itself (i.e. the dust vote supplied proposal is to transfer unallocated part to attacker's account in this case):

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

            totalTokensRequested += proposal.tokensRequested;

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

Recommended Mitigation Steps

Consider introducing the minimum accepted vote power for any proposal to be included in the final slate, as an example:

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

    function _validateSlate(uint24 distributionId_, uint256 endBlock, uint256 distributionPeriodFundsAvailable_, uint256[] calldata proposalIds_, uint256 numProposalsInSlate_) internal view returns (uint256 sum_) {
        ...

+       // using 0.1% of the total vote power used as a minimum for any winning proposal
+       uint minFundingVotePower = _distributions[distributionId_].fundingVotePowerCast / 1000;
        // 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();
+           if (proposal.fundingVotesReceived < 0 || Maths.wpow(SafeCast.toUint256(Maths.abs(proposal.fundingVotesReceived)), 2) < minFundingVotePower) revert InvalidProposalSlate();

Assessed type

Invalid Validation