Tadle

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

Loss of Funds due to wrong use of mulDiv()

Summary

Loss of Funds due to wrong use of mulDiv()

Context:
PreMarkets.sol#L672-L673

Vulnerability Details

Just as percents calculations, the deposit amount value of a certain number points to the total points is calculated by multiplying the certain number of points x the amount, and then dividing by the total points, as seen in multiple places e.g at PreMarkets.sol#L212 :

uint256 depositAmount = _points.mulDiv( <@
offerInfo.amount, <@
offerInfo.points,
Math.Rounding.Ceil
);

However, this was done wrong in the abortBidTaker() function at PreMarkets.sol#L672-L673:

uint256 depositAmount = stockInfo.points.mulDiv(
preOfferInfo.points, <@
preOfferInfo.amount, <@
Math.Rounding.Floor
);

Notice it multiplies the stockInfo points with the total points (preOfferInfo.points) instead of the amount (preOfferInfo.amount), affecting the expected deposit amount returned leading to loss of funds.

Impact

Loss of Funds due to wrong use of mulDiv()

Tools Used

Manual Review

Recommendations

Fix:

uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
+ preOfferInfo.amount,
+ preOfferInfo.points,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

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