Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Valid

Incorrect offer status check in `closeBidOffer` function prevents proper offer closure and settlement

0xsecuri

Summary

The DeliveryPlace contract's closeBidOffer function is intended to allow an offer authority to close a bid offer that is in the process of being settled. The function is designed to handle the refund of collateral and update the offer's status to Settled. However, there is a discrepancy between the expected and actual functionality of the function due to an incorrect offer status check.

Vulnerability Details

The closeBidOffer function is meant to be called when a bid offer is partially or fully executed, and the offer owner wants to finalize or close it. According to the comment above the function:

* @dev offer status must be Settling

it is logical that the offer status should be Settling to indicate that the offer has been processed in some manner.

However, the function currently checks if the offer status is Virgin:

function closeBidOffer(address _offer) external {
(
OfferInfo memory offerInfo,
MakerInfo memory makerInfo,
,
MarketPlaceStatus status
) = getOfferInfo(_offer);
​
if (_msgSender() != offerInfo.authority) {
revert Errors.Unauthorized();
}
​
if (offerInfo.offerType == OfferType.Ask) {
revert InvalidOfferType(OfferType.Bid, OfferType.Ask);
}
​
if (
status != MarketPlaceStatus.AskSettling &&
status != MarketPlaceStatus.BidSettling
) {
revert InvaildMarketPlaceStatus();
}
​
@> if (offerInfo.offerStatus != OfferStatus.Virgin) { // @audit wrong offer status check
revert InvalidOfferStatus();
}
...

The Virgin status indicates that the offer has not been "touched" or executed yet. This status check is incorrect because an offer that is being settled or has been partially executed should not be in the Virgin state.

Impact

  • Functionality Blocked: Due to this incorrect status check, offer owners will not be able to close their offers once they have started to be settled or executed. The function will revert with an InvalidOfferStatus error if the offer status is not Virgin, which is not the intended behavior.

  • Unexpected Behaviors: This could lead to unexpected behaviors, such as offers being stuck in a state where they cannot be properly closed or finalized. This issue could impact the proper handling of offers and refunds, leading to potential financial discrepancies or user frustration.

Tools Used

VSCode

Recommendations

Modify the offer status check in the closeBidOffer function to ensure it accurately reflects the expected offer status for closing.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-closeBidOffer-Virgin-Settling

Based on the current Tadle market system, the `Settling` status is never used (along with `Ongoing` and `Filled`), which is supposed to represent the state before settlement by original maker. While sementically, the `Virgin` status does not represent the correct phase to allow early closures before settlement, this issue does not have any current impact given technically the early closure of bid offers is still allowed. However, if we are basing it off of the correct status implementation (i.e. `Settling` phase appropriately updated when takers create offers), then the DoS will occur, essentially blocking any early closure of bids by subsequent makers, forcing them to follow through to final settlement. Unfortunately, none of these issues identify the correct pre-context mentioned above, but I believe medium severity is appropriate. Note for downgrade to low severity: Agree with above appeals and low severity, this is more of a status accounting error and does not have any impact, given the function of `closeBidOffer` is to withdraw the unused portion of sales proceeds. It can be executed as long as the TGE time has been reached, and it restricts the offer to be in a Virgin state. Since the statuses consistently do not utilize a switch from Vigin to Ongoing/Filled and the protocol can function appropriately even without the use of such statuses (presuming other bugs are fixed), the DoS impact will not occur.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.