Using the collateral assets' oracle price at 100% of its value to mint USDS without a fee can be used for arbitrage

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L95-L111 https://github.com/code-423n4/2024-01-salty/blob/main/src/stable/CollateralAndLiquidity.sol#L268-L287

Vulnerability details

Impact

Allowing the users to mint and borrow USDS using the collateral assets, at 100% of its value based on the oracle prices without a fee can easily be exploited by the arbitragers.

Proof of Concept

The Oracle price can not be trusted as the real-time price. Even if 3 different price feeds are used in PriceAggregator, the Deviation Threshold feature in Chainlink and the fact that Uniswap already gives a time-weighted average price will be enough to differentiate it from real-time pricing.

For example, the BTC/USD and ETH/USD Chainlink price feeds on mainnet have a Deviation threshold of 0.5%, meaning that the price will only be updated once the price movement exceeds 0.5% within the heartbeat period.

Say if the previous price point for WETH is 1000 USD, the price will only be updated once the price goes up to more than 1005 USD or down to less than 995 USD.

Below are the code snippets that show USDS being minted based on the oracle prices of the collateral assets;

function borrowUSDS( uint256 amountBorrowed ) external nonReentrant
    {
    require( exchangeConfig.walletHasAccess(msg.sender), "Sender does not have exchange access" );
    require( userShareForPool( msg.sender, collateralPoolID ) > 0, "User does not have any collateral" );
    require( amountBorrowed <= maxBorrowableUSDS(msg.sender), "Excessive amountBorrowed" );

    // Increase the borrowed amount for the user
    usdsBorrowedByUsers[msg.sender] += amountBorrowed;

    // Remember that the user has borrowed USDS (so they can later be checked for sufficient collateralization ratios and liquidated if necessary)
    _walletsWithBorrowedUSDS.add(msg.sender);

    // Mint USDS and send it to the user
    usds.mintTo( msg.sender, amountBorrowed );

    emit BorrowedUSDS(msg.sender, amountBorrowed);
    }

function maxBorrowableUSDS( address wallet ) public view returns (uint256)
    {
    // If the user doesn't have any collateral, then they can't borrow any USDS
    if ( userShareForPool( wallet, collateralPoolID ) == 0 )
        return 0;

    // The user's current collateral value will determine the maximum amount that can be borrowed
    uint256 userCollateralValue  = userCollateralValueInUSD( wallet );

    if ( userCollateralValue < stableConfig.minimumCollateralValueForBorrowing() )
        return 0;

    uint256 maxBorrowableAmount = ( userCollateralValue * 100 ) / stableConfig.initialCollateralRatioPercent();

    // Already borrowing more than the max?
    if ( usdsBorrowedByUsers[wallet] >= maxBorrowableAmount )
        return 0;

    return maxBorrowableAmount - usdsBorrowedByUsers[wallet];
    }

Tools Used

Manual Review

Recommended Mitigation Steps

Require the collateral to be locked for a certain period of time before we can borrow USDS with the collateral, or consider adding a minting fee of between 0.5% and 1% (should be higher than the deviation).

Assessed type

MEV

[Picodes]

Note that there is already a margin with the collateralization ratio

[c4-judge]

Picodes marked the issue as unsatisfactory: Insufficient proof