0xvj - forwardYield function can be sandwiched to make it distribute more yield than intended

[sherlock-admin4]

0xvj

high

forwardYield function can be sandwiched to make it distribute more yield than intended

Summary

In OCE_ZVE contract yield is distributed based on the basis returned by the fetchBasis function , which can be manipulated by an attacker by sandwiching the forwardYield call to make the locker distribute more yield than intended.

Vulnerability Detail

In forewardYield function , yield that needs to be distributed to YDL is calculated based on basis , which is the pro-rata amount of pairAsset in the pool for the lp tokens owned by the locker. But an attacker can manipulate the amount of pairAsset in the liquidity pool due to which lpBurnable will be calculated to a high value than intended value and more yield will be distributed to the user.

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);
        // @audit-issue No slippage protection while removing liquidity
        (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)));
    }

Attack steps:

1. Attacker first adds huge amount of pairAsset to the pool(buys huge amount of zve).
2. Then calls forwardYield which calculates the lpBurnable value based on pairAsset amount in the pool. As the pairAsset is inflated here lpBurnable value will also be inflated.
3. So more lp tokens will be burned than intended and more yield will be distributed to the YDL than intended yield.
4. Then the attacker sells the ZVE bought in the first step.

As the forwardYield function can be called by anyone (if block.timestamp > nextYieldDistribution) an attacker easily carry this sandwich attack using a smart contract instead of doing a sandwich attack by paying higher gas price.

Impact

More yield will be distributed to YDL than intended amount which will cause loss to the protocol.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider changing the yield distribution mechanism.

Duplicate of #296

[sherlock-admin3]

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

panprog commented:

medium, dup of #296, allows to distribute almost entire principal as if it's yield, leaving the locker with almost no funds. Since forwardYield will usually be called by keepers, who will use flashbots thus making frontrunning out of scope, this is medium. User can still call forwardYield but only if keeper didn't call it in the 12 hours prior to end of period, so this requires additional conditions.