sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

0xvj - Excess yield is granted to tranche token holders #682

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 6 months ago

0xvj

medium

Excess yield is granted to tranche token holders

Summary

The protocol fee earned by adding liquidity to uniswap through OCL_ZVE distributed to tranche token holders and DAO. But in some cases where the amount of pairAsset is increase in pool, excess rewards are given to tranche token holders.

Vulnerability Detail

Tranche token holders deposit stablecoins into tranches. The stablecoins are paired with ZVE from DAO and are used to provide liquity to uniswap through OCL_ZVE.

Any yield from uniswap protocol free is re-distributed to token holders and DAO. To account for the amount of yield added basis is being used. But the basis just accounts for the pairAsset change. So if new yield is added, the correponding LP tokens are calculated and burnt and the withdrawn pairAsset and ZVE are distributed.

    function _forwardYield(uint256 amount, uint256 lp) private nonReentrant {
        address ZVE = IZivoeGlobals_OCL_ZVE(GBL).ZVE();
        uint256 lpBurnable = (amount - basis) * lp / amount * (BIPS - compoundingRateBIPS) / BIPS;
        address pair = IFactory_OCL_ZVE(factory).getPair(pairAsset, ZVE);
        IERC20(pair).safeIncreaseAllowance(router, lpBurnable);
        (uint256 claimedPairAsset, uint256 claimedZVE) = IRouter_OCL_ZVE(router).removeLiquidity(
            pairAsset, ZVE, lpBurnable, 0, 0, address(this), block.timestamp + 14 days
        );
        emit LiquidityTokensBurned(lpBurnable, claimedZVE, claimedPairAsset);
        assert(IERC20(pair).allowance(address(this), router) == 0);
        uint balPairAsset = IERC20(pairAsset).balanceOf(address(this));
        emit YieldForwarded(pairAsset, balPairAsset);
        if (pairAsset != IZivoeYDL_OCL_ZVE(IZivoeGlobals_OCL_ZVE(GBL).YDL()).distributedAsset()) {
            IERC20(pairAsset).safeTransfer(OCT_YDL, balPairAsset);
        }
        else {
            IERC20(pairAsset).safeTransfer(IZivoeGlobals_OCL_ZVE(GBL).YDL(), balPairAsset);
        }
        IERC20(ZVE).safeTransfer(owner(), IERC20(ZVE).balanceOf(address(this)));
    }

So in this case, if the pairAsset amount increases than its considered new yield is added in the form of protocol fee. But there will be cases where the pairAsset will increase due to swaps which doesn't mean new yield is added. But in this cases also the yield is being distributed

Consider this scenario

  1. Alice adds 100 USDC to tranche.
  2. DAO deposits 100 USDC / 100 ZVE to uniswap
  3. Now due to swaps amount in the pool changes to 150 USDC / 66.66 ZVE
  4. Now the basis becomes 150 - 100 = 50 USDC
  5. So the LP tokens = 50 * 100 / 150 = 33.33
  6. So 33.33 Lp tokens are burned
  7. You get 50 USDC and 21.9 ZVE are to be distributed

The token holders are being distributed 50 USDC even is no yield is generated. This is happening due to considering only pairAsset for basis

Impact

Tranche Tokens holders get distributed yield even if there is no yield. Loss for protocol.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/lockers/OCL/OCL_ZVE.sol#L311 https://github.com/sherlock-audit/2024-03-zivoe/blob/main/zivoe-core-foundry/src/lockers/OCL/OCL_ZVE.sol#L336

Tool used

Manual Review

Recommendation

Yield should not be distributed only considering the pairAsset amount for basis

pseudonaut commented 6 months ago

Intended functionality

sherlock-admin3 commented 6 months ago

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

panprog commented:

invalid, this is intended according to developers