sherlock-audit / 2024-09-predict-fun-judging

0 stars 0 forks source link

Custom Rusty Urchin - The protocol can change the minimumOrderFeeRate after a loan offer has been created but before it has been accepted. #306

Open sherlock-admin2 opened 2 days ago

sherlock-admin2 commented 2 days ago

Custom Rusty Urchin

Medium

The protocol can change the minimumOrderFeeRate after a loan offer has been created but before it has been accepted.

Summary

Bug #1:

https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L894

In the PredictDotLoan.sol contract, the protocolFeeBasisPoints parameter represents the protocol fee rate applied during the acceptance of borrow requests and loan offers. The issue arises when the protocol fee is changed after a proposal (borrow request or loan offer) has been created, but before it has been accepted. The new protocol fee is applied retroactively to all pending proposals, including those created under the old protocolFeeBasisPoints value.

This bug can cause a significant problem, as both borrowers and lenders create proposals with the expectation that the fee they originally agreed upon will remain the same. However, if the protocol fee is changed before the offer is accepted, both parties may face unexpected costs.

Bug #2:

https://github.com/sherlock-audit/2024-09-predict-fun/blob/main/predict-dot-loan/contracts/PredictDotLoan.sol#L229-L231

In the acceptLoanOfferAndFillOrder() function of the PredictDotLoan.sol contract, the minimumOrderFeeRate parameter sets the minimum fee rate for exchange orders. The issue arises when the protocol changes the minimumOrderFeeRate after a loan offer has been created but before it has been accepted. When the borrower attempts to accept the loan offer, the contract reverts due to the offer's fee rate being lower than the new minimum rate.

This bug causes previously valid loan offers to become retroactively invalid, even though both borrowers and lenders had agreed to a specific fee rate under the old minimumOrderFeeRate.

Root Cause

Bug #1:

When protocolFeeBasisPoints is updated by the protocol, the new value is applied to all pending proposals. However, those proposals were created under the assumption that the old protocolFeeBasisPoints value would be used. This creates a discrepancy between the fee expected by the users and the actual fee applied at the time of loan acceptance.

This issue affects the flow of how loan offers and borrow requests are processed in the following manner:

A proposal is created (either borrow request or loan offer).
The protocolFeeBasisPoints is updated by the protocol.
The pending proposal is accepted, but the new protocol fee is applied, which is different from the fee the borrower or lender expected at the time of proposal creation.

Bug #2:

The contract contains a check that ensures the fee rate in the exchange order is greater than or equal to the current minimumOrderFeeRate at the time of loan offer acceptance:

if (exchangeOrder.feeRateBps < minimumOrderFeeRate) {
    revert OrderFeeRateTooLow();
}

If the minimumOrderFeeRate is changed by the protocol after the loan offer is created, but before it is accepted, this check will cause the loan offer to be invalidated, even though it was valid when created.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

Bug #1:

Retroactive changes to protocolFeeBasisPoints can lead to unexpected financial costs for borrowers and lenders when accepting previously created proposals.

Bug #2:

Changing the minimumOrderFeeRate after loan offers are created can retroactively invalidate previously valid offers, disrupting the user experience.

PoC

Bug #1:

The issue arises due to this section of the code where the current global protocolFeeBasisPoints is applied during loan acceptance:

uint256 protocolFee = (fulfillAmount * protocolFeeBasisPoints) / 10_000;

The protocolFeeBasisPoints is not locked in at the time of proposal creation, causing the issue when the global value is changed later.

Bug #2:

The issue occurs in the following code, where the current global minimumOrderFeeRate is used:

if (exchangeOrder.feeRateBps < minimumOrderFeeRate) {
    revert OrderFeeRateTooLow();
}

This check does not consider the minimumOrderFeeRate at the time of order creation, making the loan offer vulnerable to future changes in the protocol.

Mitigation

Bug #1:

Store the protocolFeeBasisPoints value in the proposal when it is created and use the stored value when the proposal is accepted, instead of the current global protocolFeeBasisPoints. This will ensure that borrowers and lenders are subject to the fee rate they agreed to at the time of proposal creation, preventing retroactive changes from affecting pending proposals.

Bug #2:

Store the minimumOrderFeeRate in the exchange order at the time of its creation. When accepting the loan offer, use the stored value to validate the fee rate, ensuring that the loan offer remains valid under the terms both parties agreed to at the time of its creation, regardless of future changes to the global minimumOrderFeeRate.

These two bugs both relate to the retroactive application of changes to protocol-level parameters (protocolFeeBasisPoints and minimumOrderFeeRate), and the recommended fixes are similar: store the relevant parameters at the time of proposal or order creation to prevent unexpected changes affecting previously created agreements.