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

Wrong math in `PreMarkets::abortBidTaker` will cause user's loss of funds.

amaron

Summary

the abortBidTaker is calculating how many tokens need to be refunded to the bid taker, however the way it is being calculated will calculate a complete different result.

Vulnerability Details

the function abortBidTaker

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 transferAmount = OfferLibraries.getDepositAmount(
preOfferInfo.offerType,
preOfferInfo.collateralRate,
depositAmount,
false,
Math.Rounding.Floor
);
​
MakerInfo storage makerInfo = makerInfoMap[preOfferInfo.maker];
ITokenManager tokenManager = tadleFactory.getTokenManager();
tokenManager.addTokenBalance(
TokenBalanceType.MakerRefund,
_msgSender(),
makerInfo.tokenAddress,
transferAmount
);
​
stockInfo.stockStatus = StockStatus.Finished;
​
emit AbortBidTaker(_offer, _msgSender());
}

calculates the depositAmount wrong, and therefore the refund amount will be wrong, and usually much lower:

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

the deposit amount, in most of the cases will be 0:
points * points / amount. (amount is with decimals), therefore, the calculated refund amount will most likely be 0, or a different number.

Impact

the wrong math might lead to severe loss of funds.

Tools Used

manual review

Recommendations

fix it to:

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

points * amount / points

Updates

Lead Judging Commences

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