Ironsidesec - Sandwich attack on `OCL_ZVE.forwardYield`

[sherlock-admin3]

Ironsidesec

medium

Sandwich attack on OCL_ZVE.forwardYield

Summary

This issue is due to the wrong slippage implementation. Cannot be fixed by access control. Still, it can be sandwiched by MEV bots. And fixing it with proper slippage implementation is the way.

Look at the pullFromLocker, it has proper slippage implemented when burning Lp

Vulnerability Detail

https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/lockers/OCL/OCL_ZVE.sol#L317

File: 2024-03-zivoe\zivoe-core-foundry\src\lockers\OCL\OCL_ZVE.sol

316:     function _forwardYield(uint256 amount, uint256 lp) private nonReentrant {
317:         address ZVE = IZivoeGlobals_OCL_ZVE(GBL).ZVE();
318:         uint256 lpBurnable = (amount - basis) * lp / amount * (BIPS - compoundingRateBIPS) / BIPS;
319:         address pair = IFactory_OCL_ZVE(factory).getPair(pairAsset, ZVE);
320:         IERC20(pair).safeIncreaseAllowance(router, lpBurnable);
321:         (uint256 claimedPairAsset, uint256 claimedZVE) = IRouter_OCL_ZVE(router).removeLiquidity(
322:    >>>        pairAsset, ZVE, lpBurnable, 0, 0, address(this), block.timestamp + 14days
323:         );
326:         emit LiquidityTokensBurned(lpBurnable, claimedZVE, claimedPairAsset);
327:         assert(IERC20(pair).allowance(address(this), router) == 0);
328:         uint balPairAsset = IERC20(pairAsset).balanceOf(address(this));
329:         emit YieldForwarded(pairAsset, balPairAsset);
330:         if (pairAsset != IZivoeYDL_OCL_ZVE(IZivoeGlobals_OCL_ZVE(GBL).YDL()).distributedAsset()) {
331:             IERC20(pairAsset).safeTransfer(OCT_YDL, balPairAsset);
332:         }
333:         else {
334:             IERC20(pairAsset).safeTransfer(IZivoeGlobals_OCL_ZVE(GBL).YDL(), balPairAsset);
335:         }
336:         IERC20(ZVE).safeTransfer(owner(), IERC20(ZVE).balanceOf(address(this)));
337:     }

Look at line 322 with 0,0 as minimum returned tokenA and tokenB amounts

Attack path:

1. If you are a Lp holder with some ZVE and USDT already provider Lp, then wait till forwardYield is called by anyone, either the keeper or anyone,
2. Then frontrun and burn the Lp with low slippage.
3. Then make the forwardYield to go through which will again burn the LP  but it will receive lesser amounts because the atatcker frontran which will decrease the worth of LP.
4. Then attacker will provide the Lp, so whatever lost by the OCL_ZVE Lp will be the attacker's gain.

Impact

Sandwich attack / MEV on OCL_ZVE.forwardYield will make OCL_ZVE receive less amounts when burning the LP. Loos may be from 1 - 50% depending on liquidity and how much is the OCL_ZVE is burning. So Medium.

Code Snippet

https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/lockers/OCL/OCL_ZVE.sol#L317

Tool used

Manual Review

Recommendation

Modify https://github.com/sherlock-audit/2024-03-zivoe/blob/01e00e6f27b58392a6fa0b82c84a46a783a0df3c/zivoe-core-foundry/src/lockers/OCL/OCL_ZVE.sol#L317

-   function forwardYield() external {
+   function forwardYield(uint minA, uint minB) external {
        if (IZivoeGlobals_OCL_ZVE(GBL).isKeeper(_msgSender())) {
            require(
                block.timestamp > nextYieldDistribution - 12 hours,
                "OCL_ZVE::forwardYield() block.timestamp <= nextYieldDistribution - 12 hours"
            );
        }
        else {
            require(
                block.timestamp > nextYieldDistribution,
                "OCL_ZVE::forwardYield() block.timestamp <= nextYieldDistribution"
            );
        }

        (uint256 amount, uint256 lp) = fetchBasis();
-       if (amount > basis) { _forwardYield(amount, lp); }  
+       if (amount > basis) { _forwardYield(amount, lp, minA, minB); }  

        (basis,) = fetchBasis();
        nextYieldDistribution += 30 days;
    }

-   function _forwardYield(uint256 amount, uint256 lp) private nonReentrant {
+   function _forwardYield(uint256 amount, uint256 lp, uint minA, uint minB) 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
+           pairAsset, ZVE, lpBurnable, minA, minB, 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)));
    }

Duplicate of #146

[sherlock-admin4]

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

panprog commented:

low, dup of #19, it is impossible to create profitable sandwitch attack for uniswap2 liquidity removal (if uniswap2 pool price is manipulated away from fair price, tokens received for liquidity in USD terms will always be greater than tokens received for liquidity at the fair price)