Tadle

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

`abortBidTaker` function calculates `depositAmount` incorrectly

Summary

Because the abortBidTaker function calculates depositAmount incorrectly, the accounting for the token amount to be refunded to the corresponding stock's authority becomes incorrect. For instance, when the calculated depositAmount is incorrectly lower than what it should be, the token amount refunded to the corresponding stock's authority would be incorrectly lower than what he is entitled to.

Vulnerability Details

The following abortBidTaker function executes uint256 depositAmount = stockInfo.points.mulDiv(preOfferInfo.points, preOfferInfo.amount, Math.Rounding.Floor), which is equivalent to depositAmount = stockInfo.points * preOfferInfo.points / preOfferInfo.amount in mathematic expression. However, since such mathematic expression does not result in a portion of preOfferInfo.amount, the calculated depositAmount does not make sense and is incorrect. Because the OfferLibraries.getDepositAmount function call uses the incorrect depositAmount to calculate transferAmount, such transferAmount is also incorrect. When the tokenManager.addTokenBalance function is called with the incorrect transferAmount for TokenBalanceType.MakerRefund, the accounting for the token amount to be refunded to the corresponding stock's authority becomes incorrect as well.

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/PreMarkets.sol#L645-L697

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;
...
}

Impact

The token amount to be refunded to the corresponding stock's authority is incorrect when the abortBidTaker function is called. For example, when the calculated depositAmount is incorrectly lower than what it should be, the token amount refunded to the corresponding stock's authority would be incorrectly lower than what he is entitled to.

Tools Used

Manual Review

Recommended Mitigation

The calculation of depositAmount in the abortBidTaker function needs to be corrected in which the correct depositAmount would be a portion of preOfferInfo.amount.

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.