Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Incorrect Refund amount due to Misordered Multiplication in abortBidTaker

cryptomoon

Summary

The function abortBidTakeris used by the takers with BIDstock and stock's preOffergot aborted. This function refunds the amount deposited by taker due to the abortion of offer. However, it calculates depositAmountwrongly which can lead to more/less refund than deposited by taker.

Vulnerability Details

The current calculation for depositAmount uses preOfferInfo.points as the multiplier and divides by preOfferInfo.amount. This is incorrect. The calculation should multiply by preOfferInfo.amount and divide by preOfferInfo.points . The current logic can lead to incorrect deposit amounts being calculated for the taker.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/PreMarkets.sol#L671-L692

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points,
preOfferInfo.amount,
Math.Rounding.Floor
); // ISSUE: it should be (stockInfo.points * preOfferInfo.amount)/preOfferInfo.points
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
​
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);

if preOfferInfo.amountis 1e6and preOfferInfo.pointsis 200e6and stockInfo.pointsis 0.5e6.

The amount deposited will be = (0.5e6 * 1e6) / 200e6 = 0.005e6

The amount refunded will be = (0.5e6 * 200e6) / 1e6 = 100e6

This will be loss to protocol.

Impact

As mentioned in the above example, this will cause loss to protocol or user depending on numbers.

Tools Used

Manual review

Recommendations

Update the logic as following:

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.amount,
preOfferInfo.points,
Math.Rounding.Floor
);
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

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

finding-PreMarkets-abortBidTaker-amount-wrong-StockInfo-points

Valid high severity, due to incorrect computation of `depositAmount` within `abortBidTaker`, when aborting bid offers created by takers, the collateral refund will be completely wrong for the taker, and depending on the difference between the value of `points` and `amount`, it can possibly even round down to zero, causing definite loss of funds. If not, if points were worth less than the collateral, this could instead be used to drain the CapitalPool contract instead.

Support

FAQs

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