Tadle

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

askMakers can abort their offers and incorrectly get their whole collateral back

Summary

The protocol implements a lot of functionality that depends on previous usage of the functions, which means if one prev functionality did not update/work properly it could affect future operations, this happens in the abortAskOfferwhere it tried to check if the OfferStatus was virgin, if it is it adds the whole collateral to the user balance, and if it is not it calculates how much they can withdraw based on the used points, but the main issue was when a taker was created in the createTakerfor that offer, it does not actually set the OfferStatus to anything but virgin, meanwhile the used points variable increases, this allows the user to abort and still get away with the whole collateral and the SalesRevenue amount

Vulnerability Details

during createTaker the used points of the offer is reduced but the offerStatus are not changed as shown below

offerInfo.usedPoints = offerInfo.usedPoints + _points;

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L164C4-L284C6

Now the user can actually call abortAskOffer to abort their offer, which implements in two ways dont allow users to leave with the whole collateral if they have used points in there offer,

uint256 remainingAmount;
if (offerInfo.offerStatus == OfferStatus.Virgin) {
remainingAmount = offerInfo.amount;
} else {
remainingAmount = offerInfo.amount.mulDiv(
offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);
}

as seen remaining amount, that can be taken is being determined by whether the status is set to virgin, or not which it still is as stated before, which allows the user to add the whole collateral amount to their balance

Impact

the essence of collateral deposit was to protect users incase of the point tokens were not settled by the askMaker, allowing the user to remove the whole collateral and also the salesRevenue defeats this idea and does not protect the takers, which they loose funds

Tools Used

Manaual Review

Recommendations

The main cause of this issue is what OfferStatus actually means in the protocol, becuase offer status is not updated anywhere but at creation, abortion, and settlement it allows actions not sanctioned to be performed across the system, this is just one of the exploit scenarios caused, OfferStatus must be updated when the first taker fill the offer to signal to future operation about the status of the offer being proccessed

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-closeBidTaker-lack-check-abort-status-drain

Valid high, for unsettled ask offers by the original maker, the initial remaining maker collateral is already refunded as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L624-L629)

Support

FAQs

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