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

Incorrect logic in abortBidTaker() will return wrong depositAmount

itsabinashb

Summary

Incorrect logic will return wrong depositAmount in PreMarkets::abortBidTaker().

Vulnerability Details

In PreMarkets::abortBidTaker() the deposit amount is calculated incorrectly. The current logic looks like this:

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

Assume, stockInfo.points = 100, preOfferInfo.points = 1000 & preOfferInfo.amount = 2000. It means price of 1 point is 2. If we follow the current implementation & put it to Math.sol::mulDiv() then we will get this:

➜ mulDiv(100,1000,2000)
Type: uint256
├ Hex: 0x32
├ Hex (full word): 0x32
└ Decimal: 50

But it is wrong, for 100 points the deposit amount should be 200 because price of 1 point is 2.

Impact

Wrong depositAmount will be calculated.

Tools Used

Manual review, Chisel

Recommendations

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

Related Links

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

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.