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

incorrect calculation of deposit amount in `PreMarkets::abortBidTaker` , leads to loss of funds for bid taker

honour

Summary

depositAmount which is the amount initially deposited by the bid taker to purchase points is wrongly calculated in preMarket::abortBidTaker as a result of this the can loose a signifcant amount (if not all) of their deposit

Vulnerability Details

PreMarkets::abortBidTaker incorrectly calculates depositAmount as seen in the code
https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L671-L675

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

depositAmount is calculated as stockPoints * offerPoints / offerAmount , which is incorrect and since points have 0 decimal precision the demoniator is way too large , so the result is most likely rounded to zero. Leading to the bid taker loosing their deposited funds.

Impact

HIGH - loss of funds for bid taker

Tools Used

Manual Review

Recommendations

function abortBidTaker(address _stock, address _offer) external {
StockInfo storage stockInfo = stockInfoMap[_stock];
OfferInfo storage preOfferInfo = offerInfoMap[_offer];
​
if (stockInfo.authority != _msgSender()) {
revert Errors.Unauthorized();
}
​
if (stockInfo.preOffer != _offer) {
revert InvalidOfferAccount(stockInfo.preOffer, _offer);
}
​
if (stockInfo.stockStatus != StockStatus.Initialized) {
revert InvalidStockStatus(
StockStatus.Initialized,
stockInfo.stockStatus
);
}
​
if (preOfferInfo.abortOfferStatus != AbortOfferStatus.Aborted) {
revert InvalidAbortOfferStatus(
AbortOfferStatus.Aborted,
preOfferInfo.abortOfferStatus
);
}
​
- uint256 depositAmount = stockInfo.points.mulDiv(
- preOfferInfo.points,
- preOfferInfo.amount,
- Math.Rounding.Floor
- );
​
+ uint256 depositAmount = stockInfo.points.mulDiv(
+ preOfferInfo.amount,
+ preOfferInfo.points,
+ Math.Rounding.Floor
+ );
​
uint256 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,//offer type can only be ask, so transferAmount == depositAmount, no point for this call
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
​
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];//@Gas
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);
​
stockInfo.stockStatus = StockStatus.Finished;
​
emit AbortBidTaker(_offer, _msgSender());
}
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.