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

Bid makers are unnable to retrieve remaining deposit after settlement if points were partially traded, resulting in funds being stuck in the contract

0xbrivan2

Summary

Bid makers are unable to retrieve their remaining deposit after settlement if their points were partially traded, due to a restriction that only allows refunds for offers in Virgin status. This results in the remaining collateral being permanently locked in the contract.

Vulnerability Details

PRE CONTEXTE: This issue arises after addressing the vulnerability where the Ongoing status is not set when an offer is taken.

Bid offer makers deposit the amount of points they are willing to purchase in advance when creating an offer using the createOffer function in PreMarket. Sellers (bid takers) then call createTaker to sell points to bid-offer makers when the market is online.

During the settlement period, bid-offer makers are expected to use the DeliveryPlace::closeBidOffer function to withdraw the collateral for the unused points—those points that were not bought. However, the function currently only allows refunds for offers that remain in the Virgin status:

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
revert InvalidOfferStatus();
}
​
// ...
}

As seen above, the check if (offerInfo.offerStatus != OfferStatus.Virgin) revert InvalidOfferStatus() restricts refunds to offers that have not been traded (i.e., Virgin status). If an offer’s points were partially used—meaning a bid taker sold some, but not all, of the offer points—and the market enters the settlement period, bid makers will be unable to retrieve the deposit for the unused points, causing these funds to remain stuck in the contract.

Impact

Bid makers are unable to retrieve the remaining deposit after settlement if their points were partially traded, resulting in their funds being permanently locked in the contract.

Tools Used

Manual Review

Recommendations

Consider updating the offer's status check in the closeBidOffer function as follows:

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) {
+ if (offerInfo.offerStatus != OfferStatus.Virgin &&
+ offerInfo.offerStatus != OfferStatus.Ongoing
+ )
revert InvalidOfferStatus();
}
​
// ...
}
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.