c4-bot-1 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/53516c2cdfdfacb662cdea6417c52f23c94d5b5b/src/stable/CollateralAndLiquidity.sol#L124-L125

Vulnerability details

Impact

Protocol Owned Liquidity from the DAO is drained as the Liquidizer doesn't have enough USDS to cover the upkeep process.

Proof of Concept

The problem is that USDS tokens are sent to the usds contract instead of the liquidizer.

// Have the user send the USDS to the USDS contract so that it can later be burned (on USDS.performUpkeep)
usds.safeTransferFrom(msg.sender, address(usds), amountRepaid);

CollateralAndLiquidity.sol#L124-L125

First of all note how the comment refers to a USDS.performUpkeep function, which doesn't exist on the USDS contract, but should refer to the upkeep in the Liquidizer contract.

The correct implementation can be seen when positions are liquidated, as they transfer assets to the Liquidizer, as well as incrementing the burnable USDS.

In contrast, the repayUSDS() function increases the burnable USDS without transfering the asset to the liquidizer (as it is transfered to the USDS contract instead):

    // Have the user send the USDS to the USDS contract so that it can later be burned (on USDS.performUpkeep)
    usds.safeTransferFrom(msg.sender, address(usds), amountRepaid);

    // Have USDS remember that the USDS should be burned
👉  liquidizer.incrementBurnableUSDS( amountRepaid );

CollateralAndLiquidity.sol#L124-L128

When incrementBurnableUSDS() is called, it increases the usdsThatShouldBeBurned variable.

This is used during the liquidizer upkeep, but as the incrementBurnableUSDS will be increased without actually sending USDS this check will return false, leading to the following action:

    // The entire usdsBalance will be burned - but there will still be an outstanding balance to burn later
    _burnUSDS( usdsBalance );
    usdsThatShouldBeBurned -= usdsBalance;

👉  // As there is a shortfall in the amount of USDS that can be burned, liquidate some Protocol Owned Liquidity and
👉  // send the underlying tokens here to be swapped to USDS
    dao.withdrawPOL(salt, usds, PERCENT_POL_TO_WITHDRAW);
    dao.withdrawPOL(dai, usds, PERCENT_POL_TO_WITHDRAW);

Liquidizer.sol#L117-L124

In short, it will "withdraw Protocol Owned Liquidity from the DAO" as described here to cover the shortage of USDS in the Liquidizer contract, slowly draining the POL of the DAO.

Recommended Mitigation Steps

Send the assets to the liquidizer:

-   // Have the user send the USDS to the USDS contract so that it can later be burned (on USDS.performUpkeep)
-   usds.safeTransferFrom(msg.sender, address(usds), amountRepaid);
+   // Have the user send the USDS to the Liquidizer contract so that it can later be burned (on Liquidizer.performUpkeep)
+   usds.safeTransferFrom(msg.sender, address(liquidizer), amountRepaid);