Timenov - Bad implementation of yield distribution can leave users with no yield.

[sherlock-admin3]

Timenov

medium

Bad implementation of yield distribution can leave users with no yield.

Summary

Yield can be distributed by calling ZivoeYDL::distributeYield. This can be called by anyone as long as unlocked == true and block.timestamp >= lastDistribution + daysBetweenDistributions * 86400. Then the function will send distributedAsset to all addresses part of protocolRecipients and residualRecipients. Several problems can occur because of bad implementation.

Vulnerability Detail

I will list 3 problems that will break the logic(listed by most likely to happen).

Problem 1(Out of gas)

This function is pretty big. It has 2 for loops that iterate over protocolRecipients and residualRecipients. In each iteration several calls are performed like safeIncreaseAllowance, depositReward, safeTransfer and many others. Also require statements and math operations. This easily can run out of gas.

Yes, there is a function updateRecipients that will updated protocolRecipients and residualRecipients, but still the issue is there. Lets say that call fails with 100 recipients. The recipients are split by 2, so now we have 2 groups of 50. The first will receive their yield, but the second group will have to wait more time. This can happen multiple times, not only once.

Problem 2(User can not receive distributedAsset)

If a user that is part of either protocolRecipients or residualRecipients can not receive distributedAsset(e.g. part of USDC blacklist), the function will fail and no one will receive yield.

Problem 3(User blocking distribution by changing lastDistribution)

Lets say DAI was distributedAsset and is successfully sent to everyone. Now the next will be USDC. However it is still not updated, but the block.timestamp >= lastDistribution + daysBetweenDistributions * 86400 is passed. Now DAI will still be the distributedAsset, but the balance will be 0. User can call distributeYield just to increase the lastDistribution in order to block yield distribution to a certain period.

Impact

Yield distribution will be blocked.

Code Snippet

    function distributeYield() external nonReentrant {
        require(unlocked, "ZivoeYDL::distributeYield() !unlocked"); 
        require(
            block.timestamp >= lastDistribution + daysBetweenDistributions * 86400, 
            "ZivoeYDL::distributeYield() block.timestamp < lastDistribution + daysBetweenDistributions * 86400"
        );

        // Calculate protocol earnings.
        uint256 earnings = IERC20(distributedAsset).balanceOf(address(this));
        uint256 protocolEarnings = protocolEarningsRateBIPS * earnings / BIPS;
        uint256 postFeeYield = earnings.floorSub(protocolEarnings);

        // Update timeline.
        distributionCounter += 1;
        lastDistribution = block.timestamp;

        // Calculate yield distribution (trancheuse = "slicer" in French).
        (
            uint256[] memory _protocol, uint256 _seniorTranche, uint256 _juniorTranche, uint256[] memory _residual
        ) = earningsTrancheuse(protocolEarnings, postFeeYield); 

        emit YieldDistributed(_protocol, _seniorTranche, _juniorTranche, _residual);

        // Update ema-based supply values.
        (uint256 aSTT, uint256 aJTT) = IZivoeGlobals_YDL(GBL).adjustedSupplies();
        emaSTT = MATH.ema(emaSTT, aSTT, retrospectiveDistributions.min(distributionCounter));
        emaJTT = MATH.ema(emaJTT, aJTT, retrospectiveDistributions.min(distributionCounter));

        // Distribute protocol earnings.
        for (uint256 i = 0; i < protocolRecipients.recipients.length; i++) {
            address _recipient = protocolRecipients.recipients[i];
            if (_recipient == IZivoeGlobals_YDL(GBL).stSTT() ||_recipient == IZivoeGlobals_YDL(GBL).stJTT()) {
                IERC20(distributedAsset).safeIncreaseAllowance(_recipient, _protocol[i]);
                IZivoeRewards_YDL(_recipient).depositReward(distributedAsset, _protocol[i]);
                emit YieldDistributedSingle(distributedAsset, _recipient, _protocol[i]);
            }
            else if (_recipient == IZivoeGlobals_YDL(GBL).stZVE()) {
                uint256 splitBIPS = (
                    IERC20(IZivoeGlobals_YDL(GBL).stZVE()).totalSupply() * BIPS
                ) / (
                    IERC20(IZivoeGlobals_YDL(GBL).stZVE()).totalSupply() + 
                    IERC20(IZivoeGlobals_YDL(GBL).vestZVE()).totalSupply()
                );
                uint stZVEAllocation = _protocol[i] * splitBIPS / BIPS;
                uint vestZVEAllocation = _protocol[i] * (BIPS - splitBIPS) / BIPS;
                IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).stZVE(), stZVEAllocation);
                IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).vestZVE(),vestZVEAllocation);
                IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).stZVE()).depositReward(distributedAsset, stZVEAllocation);
                IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).vestZVE()).depositReward(distributedAsset, vestZVEAllocation);
                emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).stZVE(), stZVEAllocation);
                emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).vestZVE(), vestZVEAllocation);
            }
            else {
                IERC20(distributedAsset).safeTransfer(_recipient, _protocol[i]);
                emit YieldDistributedSingle(distributedAsset, _recipient, _protocol[i]);
            }
        }

        // Distribute senior and junior tranche earnings.
        IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).stSTT(), _seniorTranche);
        IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).stJTT(), _juniorTranche);
        IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).stSTT()).depositReward(distributedAsset, _seniorTranche);
        IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).stJTT()).depositReward(distributedAsset, _juniorTranche);
        emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).stSTT(), _seniorTranche);
        emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).stJTT(), _juniorTranche);

        // Distribute residual earnings.
        for (uint256 i = 0; i < residualRecipients.recipients.length; i++) {
            if (_residual[i] > 0) {
                address _recipient = residualRecipients.recipients[i];
                if (_recipient == IZivoeGlobals_YDL(GBL).stSTT() ||_recipient == IZivoeGlobals_YDL(GBL).stJTT()) {
                    IERC20(distributedAsset).safeIncreaseAllowance(_recipient, _residual[i]);
                    IZivoeRewards_YDL(_recipient).depositReward(distributedAsset, _residual[i]);
                    emit YieldDistributedSingle(distributedAsset, _recipient, _protocol[i]);
                }
                else if (_recipient == IZivoeGlobals_YDL(GBL).stZVE()) {
                    uint256 splitBIPS = (
                        IERC20(IZivoeGlobals_YDL(GBL).stZVE()).totalSupply() * BIPS
                    ) / (
                        IERC20(IZivoeGlobals_YDL(GBL).stZVE()).totalSupply() + 
                        IERC20(IZivoeGlobals_YDL(GBL).vestZVE()).totalSupply()
                    );
                    uint stZVEAllocation = _residual[i] * splitBIPS / BIPS;
                    uint vestZVEAllocation = _residual[i] * (BIPS - splitBIPS) / BIPS;
                    IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).stZVE(), stZVEAllocation);
                    IERC20(distributedAsset).safeIncreaseAllowance(IZivoeGlobals_YDL(GBL).vestZVE(), vestZVEAllocation);
                    IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).stZVE()).depositReward(distributedAsset, stZVEAllocation);
                    IZivoeRewards_YDL(IZivoeGlobals_YDL(GBL).vestZVE()).depositReward(distributedAsset, vestZVEAllocation);
                    emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).stZVE(), stZVEAllocation);
                    emit YieldDistributedSingle(distributedAsset, IZivoeGlobals_YDL(GBL).vestZVE(), vestZVEAllocation);
                }
                else {
                    IERC20(distributedAsset).safeTransfer(_recipient, _residual[i]);
                    emit YieldDistributedSingle(distributedAsset, _recipient, _residual[i]);
                }
            }
        }
    }

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/ZivoeYDL.sol#L213-L311

Tool used

Manual Review

Recommendation

Consider implementing the following changes:

Problem 1 fix:

Use pull over push. Add mapping that will keep track of yield for each user. Add another function that users will call to get their yield.

Problem 2 fix:

Add parameter recipient to the new function, that will allow blacklisted users to send their yield to the address.

Problem 3 fix:

Add a check to verify that distributedAsset is different from the previous one or restrict only trusted entities to call that function.

[pseudonaut]

Not relevant imho

[sherlock-admin4]

1 comment(s) were left on this issue during the judging contest.

panprog commented:

invalid. Please report each issue independantly next time. Problem 1: The recepients list is supposed to be pretty small, just some trusted addresses set by the admins, so this is by-design. The gas problem is purely speculation: if the recepients list is small, there is no problem with it. Problem 2: since recipients are set by trusted admin, the addresses are highly unlikely to become blacklisted, but even if it happens, there is no problem for the admin to update the recepients to fix this issue. Problem 3: Don't see any problem or impact here. If distributedAsset is updated, all the calculations are kept from the previous one, just everything is now paid in a different asset. No problem there.