newspacexyz - Reverting in acceptLoanOfferAndFillOrder() due to feeRateBps >= 10000

[sherlock-admin4]

newspacexyz

Medium

Reverting in acceptLoanOfferAndFillOrder() due to feeRateBps >= 10000

Summary

There is no validation of maximum exchangeOrder.feeRateBps. So the acceptLoanOfferAndFillOrder() function may revert if the exchangeOrder.feeRateBps is greater than 10000 basis points (which is equivalent to 100%). https://github.com/sherlock-audit/2024-09-predict-fun-dustinhuel2/blob/41e70f9eed3f00dd29aba4038544150f5b35dccb/predict-dot-loan/contracts/PredictDotLoan.sol#L214-L315

Root Cause

The fee rate in the order (feeRateBps) can exceed 10000 basis points (100%).

Internal pre-conditions

The exchangeOrder.feeRateBps in the order is set to a value greater than 10000 (i.e., more than 100%).

External pre-conditions

A user submits an exchangeOrder with feeRateBps greater than 10000 and the transaction attempts to calculate and apply the fee, which results in an error, potentially leading to a revert.

Attack Path

A user or malicious actor submits an exchangeOrder with an intentionally high feeRateBps value (e.g., 20000, representing 200%). The contract processes the order without validating or capping the fee rate. The function calculates the fee using the high feeRateBps, leading to a transaction failure due to unexpected computation results (such as overflows or exceeding acceptable limits). The transaction reverts, preventing legitimate order fulfillment.

Impact

Failed Transactions: Users will experience reverted transactions if they attempt to submit orders with a feeRateBps greater than 10000. This could disrupt loan processing and frustrate users. DoS Risk: Although it is not a direct Denial-of-Service (DoS) attack, the protocol may experience temporary service disruption if many orders are submitted with excessively high feeRateBps, leading to frequent transaction failures.

PoC

function testFuzz_acceptLoanOfferAndFillOrder_NonZeroOrderFeeRateWithZeroMinimumOrderFeeRate(
    uint16 orderFeeRateBps
) public {
    vm.assume(orderFeeRateBps > 0 && orderFeeRateBps <= 50);

    uint8 protocolFeeBasisPoints = 50;

    _updateProtocolFeeRecipientAndBasisPoints(protocolFeeBasisPoints);

    vm.prank(owner);
    predictDotLoan.updateMinimumOrderFeeRate(0);

    Order memory order = _createMockCTFSellOrder();
    order.feeRateBps = 10001;
    IPredictDotLoan.Proposal memory proposal = _generateLoanOffer(IPredictDotLoan.QuestionType.Binary);
    uint256 protocolFee = (order.takerAmount * protocolFeeBasisPoints) / 10_000;
    proposal.loanAmount = order.takerAmount + protocolFee;
    proposal.signature = _signProposal(proposal);

    vm.prank(borrower);
    predictDotLoan.acceptLoanOfferAndFillOrder(order, proposal);
}

By order.feeRateBps = 10001, this transaction reverts.

Mitigation

Add a validation step inside the acceptLoanOfferAndFillOrder() function to ensure that exchangeOrder.feeRateBps is within a reasonable range. Specifically, the fee rate should be capped at 10000 basis points (representing 100%).

require(exchangeOrder.feeRateBps <= 10000, "Fee rate cannot exceed 10000 basis points (100%)");