sherlock-audit / 2024-03-zivoe-judging

7 stars 5 forks source link

blackhole - The pushToLockerMulti function will revert upon calling the addLiquidity function. #317

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

blackhole

high

The pushToLockerMulti function will revert upon calling the addLiquidity function.

Summary

After invoking the addLiquidity, the allowance of pairAsset couldn't be zero. The token allowance is a desired amount for the liquidity addition. However, the addLiquidity function doesn't automatically reset the allowance to zero after using the tokens.

Vulnerability Detail

Therefore, the token allowance cannot be zero. src/lockers/OCL/OCL_ZVE.sol:L208-L209

Impact

This means that pushToLockerMulti won't work because the token allowance won't be set to zero as expected.

Code Snippet

src/lockers/OCL/OCL_ZVE.sol:L197-L209

        // Prevent volatility of greater than 10% in pool relative to amounts present.
        (uint256 depositedPairAsset, uint256 depositedZVE, uint256 minted) = IRouter_OCL_ZVE(router).addLiquidity(
            pairAsset, 
            ZVE, 
            balPairAsset,
            balZVE, 
            (balPairAsset * 9) / 10,
            (balZVE * 9) / 10, 
            address(this), block.timestamp + 14 days
        );
        emit LiquidityTokensMinted(minted, depositedZVE, depositedPairAsset);
        assert(IERC20(pairAsset).allowance(address(this), router) == 0); // @audit reverted?
        assert(IERC20(ZVE).allowance(address(this), router) == 0); // @audit reverted?

Tool used

Manual Review

Recommendation

Removing the check that relies on the allowance being zero and instead setting the allowance to zero after calling addLiquidity.

Duplicate of #18

sherlock-admin3 commented 4 months ago

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

panprog commented:

medium, dup of #18. While amounts provided are from the trusted admin, who is supposed to provide exact amounts to add liquidity in correct ratio of token amounts, the fact that live balance of the contract is used makes it possible for the balance to change from the time admin provides the amounts, making the transaction revert. Additionally, current contract balances might be in so high disproportion, that admin will simply not have enough funds to create correct ratio to add liquidity.