Tadle

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

Incorrect Formula Causes Protocol to Refund an Incorrect Amount, Leading to the Same Value for `remainingAmount` and `totalUsedAmount`

Summary

An incorrect formula in the calculation of the remainingAmount and totalUsedAmount variables in the PreMarkets::abortAskOffer function causes the makerRefundAmount sent to the Maker to be inaccurate. This leads to improper refunds, potentially resulting in zero or incorrect refunds being issued.

Vulnerability Details

The vulnerability exists in the PreMarkets::abortAskOffer function, where both remainingAmount and totalUsedAmount are calculated using the same formula, even though they are intended to represent opposite values.

Problematic Code:

  1. Total Used Amount Calculation:

    uint256 totalUsedAmount = offerInfo.amount.mulDiv(
    offerInfo.usedPoints,
    offerInfo.points,
    Math.Rounding.Ceil
    );
  2. Remaining Amount Calculation:

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

The above formula is calculating the usedAmount, not the remainingAmount. remainingAmount is calculated as:

remainingAmount = offerInfo.amount - usedAmount

The correct thing is done in OfferLibraries::getRefundAmount

Issue Breakdown:

  • Both remainingAmount and totalUsedAmount are calculated using the same formula, despite being intended to represent different values.

  • As a result, they end up being the same value, which then leads to the transferAmount and totalDepositAmount being calculated as the same value.

Impact on Refunds:

  • Transfer Amount Calculation:

    uint256 transferAmount = OfferLibraries.getDepositAmount(
    offerInfo.offerType,
    offerInfo.collateralRate,
    remainingAmount,
    true,
    Math.Rounding.Floor
    );
  • Total Deposit Amount Calculation:

    uint256 totalDepositAmount = OfferLibraries.getDepositAmount(
    offerInfo.offerType,
    offerInfo.collateralRate,
    totalUsedAmount,
    false,
    Math.Rounding.Ceil
    );
  • Since remainingAmount and totalUsedAmount are incorrectly calculated to be the same, transferAmount and totalDepositAmount also end up being the same, leading to makerRefundAmount being zero or another incorrect value depending on the rounding direction.

    if (transferAmount > totalDepositAmount) {
    makerRefundAmount = transferAmount - totalDepositAmount;
    } else {
    makerRefundAmount = 0;
    }

This leads to incorrect or zero refunds to the Maker, which is problematic as it may result in financial losses or unintended reverts.

Impact

Due to the incorrect formula, the protocol consistently miscalculates the refund amounts, potentially leading to zero refunds or incorrect token amounts being returned to the Maker when they abort. This could erode user trust and result in financial discrepancies within the protocol.

Tools Used

  • Manual analysis

Recommendations

  • Correct the Calculation Formulas:
    Ensure that the formulas used to calculate remainingAmount and totalUsedAmount are distinct and correctly reflect their intended values. This will prevent them from being the same and will ensure accurate refunds to Makers.

  • Review the Logic for Refund Calculations:
    Consider revisiting the refund logic to ensure that rounding errors and similar issues do not lead to incorrect refunds. Testing with various rounding methods can help in identifying potential edge cases that may need addressing.

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.