code-423n4 / 2024-01-salty-findings

11 stars 6 forks source link

Inefficient Utilization of Protocol Liquidity due to Swap Limits in Liquidizer Contract #652

Open c4-bot-10 opened 9 months ago

c4-bot-10 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/pools/PoolUtils.sol#L54 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/Liquidizer.sol#L101 https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/Liquidizer.sol#L132

Vulnerability details

Vulnerability Details:

The Liquidizer contract is integral for converting tokens from collateral liquidations and Protocol Owned Liquidity (POL) withdrawals into USDS, which is subsequently burned. However, an inefficiency arises in its performUpkeep function due to the internal swap limit set by _placeInternalSwap. This function imposes a maximum amount that can be swapped, calculated as a percentage of the token reserves in the liquidity pool.

function _placeInternalSwap(
        IPools pools,
        IERC20 tokenIn,
        IERC20 tokenOut,
        uint256 amountIn,
        uint256 maximumInternalSwapPercentTimes1000
    ) internal returns (uint256 swapAmountIn, uint256 swapAmountOut) {
        if (amountIn == 0) {
            return (0, 0);
        }

        (uint256 reservesIn,) = pools.getPoolReserves(tokenIn, tokenOut);

        uint256 maxAmountIn = reservesIn * maximumInternalSwapPercentTimes1000 / (100 * 1000);
        if (amountIn > maxAmountIn) {
            amountIn = maxAmountIn;
        }

        swapAmountIn = amountIn;

        swapAmountOut = pools.depositSwapWithdraw(tokenIn, tokenOut, amountIn, 0, block.timestamp);
    }

The core issue is that even if the Liquidizer contract holds enough tokens to cover the usdsThatShouldBeBurned amount, the swap limit might prevent converting the entire token balance into USDS. This limitation can lead to an unnecessary withdrawal of POL and its subsequent conversion to USDS, despite the contract already possessing sufficient funds.

Impact:

This inefficiency can lead to the unnecessary liquidation of POL assets. It results in a sub-optimal utilization of the protocol's liquidity and might cause unintended market impacts due to the additional conversion activities.

Tools Used:

Recommendation:

consider implementing a mechanism within the performUpkeep function to assess whether the total value of the tokens in the Liquidizer contract (after applying the swap) is sufficient to cover the usdsThatShouldBeBurned amount. If it is, then avoid withdrawing from POL and the next performUpkeep call can cover the debt.

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as primary issue

c4-sponsor commented 9 months ago

othernet-global (sponsor) acknowledged

Picodes commented 8 months ago

I don't see while buying back USDS would be an issue even if it's not strictly needed

c4-judge commented 8 months ago

Picodes changed the severity to QA (Quality Assurance)