Incomplete Price Range Validation in validateStopPrice Function

[howlbot-integration]

Lines of code

https://github.com/code-423n4/2024-05-predy/blob/a9246db5f874a91fb71c296aac6a66902289306a/src/markets/perp/PerpMarketLib.sol#L138-L176

Vulnerability details

Proof of Concept

The vulnerability in question pertains to the validateStopPrice function within the PerpMarketLib contract, which oversees the validation of trade prices using the Bps library's upper and lower functions. Here's a detailed analysis of how this vulnerability manifests and its implications:

Code Snippet Analysis:
PerpMarketLib.sol#L138-L176

 function validateStopPrice(
        uint256 oraclePrice,
        uint256 tradePrice,
        int256 tradeAmount,
        uint256 stopPrice,
        bytes memory auctionData
    ) internal pure returns (bool) {
        AuctionParams memory auctionParams = abi.decode(auctionData, (AuctionParams));

        uint256 decayedSlippageTorelance = DecayLib.decay2(
            auctionParams.startPrice,
            auctionParams.endPrice,
            auctionParams.startTime,
            auctionParams.endTime,
            ratio(oraclePrice, stopPrice)
        );

        if (tradeAmount > 0) {
            // buy
            if (stopPrice > oraclePrice) {
                return false;
            }

            if (tradePrice > Bps.upper(oraclePrice, decayedSlippageTorelance)) {
                return false;
            }
        } else if (tradeAmount < 0) {
            // sell
            if (stopPrice < oraclePrice) {
                return false;
            }

            if (tradePrice < Bps.lower(oraclePrice, decayedSlippageTorelance)) {
                return false;
            }
        }

        return true;
    }

1. $$Conditional Validation$$: The validateStopPrice function checks either the upper (Bps.upper) or lower (Bps.lower) price bounds based on the tradeAmount parameter:

• When tradeAmount > 0 (indicating a buy order), it validates the stop price (stopPrice) against oraclePrice and checks if tradePrice exceeds the upper bound (Bps.upper(oraclePrice, decayedSlippageTorelance)).
• When tradeAmount < 0 (indicating a sell order), it validates the stop price (stopPrice) against oraclePrice and checks if tradePrice falls below the lower bound (Bps.lower(oraclePrice, decayedSlippageTorelance)).

2. $$Single Bound Validation$$: The function only validates one bound (either upper or lower) based on the direction of the trade (tradeAmount), not both simultaneously. This approach introduces a vulnerability where:

• Buy Orders: If tradeAmount > 0 and tradePrice exceeds the upper bound, the function might allow trades to proceed at prices higher than intended, potentially leading to overpayment by buyers.
• Sell Orders: Conversely, if tradeAmount < 0 and tradePrice falls below the lower bound, the function might permit trades at prices lower than intended, resulting in underpayment to sellers.

Impact

Allows trades to be executed at prices outside of the intended range when only one price bound (upper or lower) is validated

Tools Used

Manual

Recommended Mitigation Steps

Modify the validateStopPrice function to validate both the upper and lower price bounds regardless of the direction of the trade (tradeAmount).

Assessed type

Other

[c4-judge]

alex-ppg marked the issue as unsatisfactory:
Invalid

[Odhiambo526]

@alex-ppg mind making a precise description on why this issue was marked invalid?

[alex-ppg]

Please see the primary submission's ruling #71 (comment)