Tadle

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

Incorrect Offer Status Check in `DeliveryPlace.sol::closeBidOffer` Function

# [M-3] Incorrect Offer Status Check in `DeliveryPlace.sol::closeBidOffer` Function
## Summary
The `closeBidOffer` function contains an incorrect status check for the `offerStatus`, which can lead to unauthorized closure of bid offers. This discrepancy between the function logic and the documented comments may result in premature offer closures, leading to potential financial losses and contract state inconsistencies.
## Vulnerability Details
The `closeBidOffer` function is intended to close a bid offer if the following conditions are met:
1. The caller is the offer authority.
2. The offer type is `Bid`.
3. The marketplace status is `AskSettling` or `BidSettling`.
4. The offer status is `Settling`.
However, the current implementation checks if the `offerStatus` is `Virgin` instead of `Settling`, which contradicts the function comments and intended logic.
## Impact
The incorrect status check may allow offers that are not in the `Settling` state to be closed, potentially leading to:
1. Unauthorized refunds.
2. Inconsistent contract states.
3. Financial losses for users who have their offers closed prematurely or incorrectly.
## Tools Used
- Manual code review
## Recommendations
Modify the status check to align with the comments and intended logic. Ensure that the offer can only be closed if it is in the `Settling` state.
## Severity
**Impact**: Medium
**Likelihood**: Medium
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.