Tadle

Tadle
DeFi
30,000 USDC
View results
Submission Details
Severity: low
Invalid

`abortAskOffer` will not return any collateral if the `OfferStatus.Virgin is false `because of wrong calculation

Summary

In PreMarkets.sol::abortAskOffer() If the offer is in a virgin state, the refund amount will be the entire offer amount. But if the offer is in a canceled state, the refund amount will be calculated as the offer amount multiplied by the used points divided by the total points, rounded down to the nearest whole number .The result will be saved as remainingAmount . Then remainingAmount will be used for calculating transferAmount by calling getDepositAmount() . Then we have totalUsedAmount which will be calculated the same way as remainingAmount and it will have the same value as him. Then we have another variable called totalDepositAmount which will return the result from calling getDepositAmount() by passing in the value of totalUsedAmount and will return again the same result as remainingAmount and totalUsedAmount . At the end it will jump into the if check where

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

and will return the result in the else block because of wrong calculations.

Vulnerability Details

Lets look at the following example:

  1. Alice is using a Turbo mode to list offer

  2. Alice, a market maker, lists 1,000 points for sale at 1$ per unit and deposits 1000$ as collateral.

  3. Bob, a buyer, purchases 500 points from Alice for 500. Alice`s Taddle board shows 500$ credited and available for withdrawal. Bob becomes the holder of 500 points.

  4. Alice decides to abort her offer and calls abortAskOffer()
    I) The if (offerInfo.offerStatus == OfferStatus.Virgin) in the function will not be true and will jump in the else block
    II) remainingAmount will be calculated as offer amount multiplied by the used points divided by the total points which is (1000 * 500) / 1000 = 500
    III) Then transferAmount will be 5 because in getDepositAmount() _amount will be remainingAMountandisMaker is set to true` then it will be calculated as:

Math.mulDiv(
_amount,
_collateralRate, // lets say 100% just for the example
Constants.COLLATERAL_RATE_DECIMAL_SCALER, // its set by default to 10_000
_rounding`

which is equal to (500*100)/10_000 = 5

IV) totalUsedAmount will be calculted the same way as remainingAmount and will have the same value of 500
V) totalDepositAmount will be calculated with getDepositAmount() passing for value _amount = totalUsedAmount and isMaker = false which will give the result of 500
VI) When it goes to the if (transferAmount > totalDepositAmount) = 5 > 500 which will be false and will set in the else block makerRefundAmount = 0
5. Alice loses the rest of collateral she hasnt sold

Impact

Loss of funds for the msg.caller if OfferStatus.Virgin is false because of wrong calculation

Tools Used

Manual Review

Recommendations

For calculating the makerRefundAmount subtract calculate the leftovers leftovers = totalPoints - usedPoints and use the following calculation

uint256 makerRefundAmount= leftovers.mulDiv(
offerInfo.points,
offerInfo.amount,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

0xnevi Lead Judge 10 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 10 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.