Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Wrong calculation depositAmount in the abortBidTaker function of PreMarkets.sol

danielwang8824

Summary

When call abortBidTaker in the PreMarkets.sol, the depositAmount is calculated wrong.

Vulnerability Details

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

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

In the above calculation, the above formular is wrong, but the below formular is correct

depositAmount = Floor ( stockInfo.points * preOfferInfo.amount / preOfferInfo.points ) // <--- this is correct

Impact

This wrong calculation cause wrong balance value generation, so user could not get wrong refund amount.

Tools Used

Manual Review

Recommendations

In PreMarkets.sol,

function abortBidTaker(address _stock, address _offer) external {
...
uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
+ preOfferInfo.amount,
+ preOfferInfo.points,
Math.Rounding.Floor
);
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year 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.