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

Incorrect remainingAmount Calculation for Non-Virgin Status

cheatcode

Summary

The abortAskOffer function is designed to handle the financial settlement when an ask offer is aborted, including calculating and processing refunds. It performs several checks to ensure the caller has the authority to abort the offer and that the offer is in a valid state for abortion. It then calculates the amounts to be refunded based on the current state of the offer.

The problem in this function lies in the calculation of the remainingAmount and its subsequent use in determining the transferAmount and makerRefundAmount.

Code Section

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

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

When the offer status is not Virgin, the remainingAmount is calculated using usedPoints. However, this calculation actually represents the amount that has been used, not the amount remaining. This is a logical error that inverts the intended meaning of remainingAmount.

This incorrectly calculated remainingAmount is then used to determine the transferAmount:

uint256 transferAmount = OfferLibraries.getDepositAmount(
offerInfo.offerType,
offerInfo.collateralRate,
remainingAmount,
true,
Math.Rounding.Floor
);

The transferAmount is critical as it's used to calculate the makerRefundAmount.

Impact

Users may receive incorrect refunds, leading to potential losses or unintended gains.

Mitigation

The correct calculation for remainingAmount when the offer is not in Virgin status should be:

remainingAmount = offerInfo.amount.mulDiv(
offerInfo.points - offerInfo.usedPoints,
offerInfo.points,
Math.Rounding.Floor
);

This calculates the truly remaining amount by subtracting the used points from the total points.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months 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 11 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.