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

Collateral tokens are incorrectly refunded in `PreMarkets.abortAskOffer()`

baz1ka

Relevant GitHub Links

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

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

Summary

PreMarkets.abortAskOffer() allows offer owner to close offer and refund unused collateral tokens. Because of incorrect calculation, user would get fewer collateral tokens back than it should be.

Vulnerability Details

To get refund amount we need to know 2 values: total deposited collateral amount and collateral amount that is already used. Let's take a look at block which determines remaining amount of collateral:

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

If offer status is OfferStatus.Virgin then no points were used and remainingAmount is all amount of collateral that was deposited. If offer status is OfferStatus.Canceled then we should get difference between all tokens and used tokens but in condition statement, we only get the collateral amount of used points which is wrong.

Impact

Offer owner can not get all unused collateral tokens back when aborting ask offer.

Recommended Mitigation Steps

Change calculation of else block to this:

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

Lead Judging Commences

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

[invalid] finding-PreMarkets-abortAskOffer-remainingAmount-compute

Valid high, for cancelled offers, the unused collateral should be returned back to the maker. The `remainingAmount` is calculated wrongly with regards to usedPoints instead of unused points. Note: See comments under 826 and 907 for invalidation reasons

Appeal created

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-PreMarkets-abortAskOffer-remainingAmount-compute

Valid high, for cancelled offers, the unused collateral should be returned back to the maker. The `remainingAmount` is calculated wrongly with regards to usedPoints instead of unused points. Note: See comments under 826 and 907 for invalidation reasons

Support

FAQs

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