sherlock-audit / 2024-03-zivoe-judging

8 stars 6 forks source link

denzi_ - `removeLiquidity` when forwarding yield in OCL_ZVE contract can cause loss of funds for the protocol due to hardcoded 0 for min PairAsset and ZVE #688

Closed sherlock-admin3 closed 6 months ago

sherlock-admin3 commented 6 months ago

denzi_

high

removeLiquidity when forwarding yield in OCL_ZVE contract can cause loss of funds for the protocol due to hardcoded 0 for min PairAsset and ZVE

Summary

The _forwardYield function within the OCL_ZVE contract utilizes a removeLiquidity call with both amountAMin and amountBMin parameters set to zero. This configuration exposes the contract to risks of market volatility and price manipulation, potentially leading to significant financial losses. The extended deadline of 14 days could potentially expose the transaction to prolonged market volatility, increasing the risk of price movements.

Vulnerability Detail

In the removeLiquidity function call, the absence of minimum thresholds (0 for both amountAMin and amountBMin) for the assets withdrawn allows the transaction to proceed regardless of the market conditions. This means the contract could receive far less value than expected or virtually nothing in return for its liquidity tokens, especially in volatile or manipulated market scenarios.

(uint256 claimedPairAsset, uint256 claimedZVE) = IRouter_OCL_ZVE(router)
            .removeLiquidity(
                pairAsset,
                ZVE,
                lpBurnable,
                0, // min pair //@audit-issue 0 min
                0, // min ZVE //@audit-issue 0 min
                address(this),
                block.timestamp + 14 days
            );

Impact

When this call will execute, the protocol can lose significant or even all of the lpBurnable tokens in return for nothing. In a volatile market, the protocol is sure to be effected by the current minimum amounts set. The protocol will lose the yield allocated for the interactors of the protocol.

Code Snippet

_forwardYield()

Tool used

Manual Review

Recommendation

Implement proper minimum amounts for pairAsset and ZVE. Consider applying at least 90-95% or take values from the caller for the minimum amounts to ensure that the protocol receives fair value for withdrawn liquidity.

Duplicate of #146

sherlock-admin4 commented 6 months ago

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)