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

Incorrect Deposit Amount Calculation in abortBidTaker

aksoy

Summary

The abortBidTaker function contains a vulnerability where the deposit amount is incorrectly calculated when aborting a bid. The calculation method used during the abort process does not match the method used when users initially deposit funds.

Vulnerability Details

Deposit amount for the taker is calculated based on the points rate during the initial deposit as follows:

/// @dev Transfer token from user to capital pool as collateral
uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);

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

However, when users abort taker bids, the deposit amount is incorrectly calculated not using points rate but instead multiplying points.

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

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

Impact

Users may receive less or more than the correct amount, leading to potential losses for the protocol or users.

Tools Used

Manual

Recommendations

Use the correct method to calculate deposit amount for users.

- uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
- Math.Rounding.Floor
- );
​
​
+ uint256 depositAmount = stockInfo.points.mulDiv(
+ 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.