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

Vulnerability in PreMarktes::abortBidTaker Function Leading to Incorrect Taker Deposit Calculation

josh4324

Summary

The abortBidTaker function is intended to allow the Taker to reclaim their deposited funds if the Maker aborts an offer after the Taker has already made a payment. However, a critical issue has been identified in the function's calculation of the Taker’s deposited funds. Due to an incorrect calculation method and precision loss, the Taker often ends up with 0 tokens, resulting in the funds being effectively stuck within the contract. This vulnerability can cause significant financial losses for the Taker and impacts the reliability of the contract.

Vulnerability Details

The abortBidTaker function uses an incorrect formula to calculate the amount of funds that the Taker should receive back after the Maker aborts the offer. The calculation fails to account for certain factors, leading to an inaccurate result that often rounds down to 0 due to precision loss. Due to the incorrect calculation and precision loss, the Taker’s funds become stuck in the contract. The Taker is unable to retrieve these funds through the normal process, leading to a situation where the deposited tokens are effectively locked away with no recourse for recovery.

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);
}
​
// Audit wrong calculation
@> 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());
}

POC

function test_abortBidTaker() public {
// create offer
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
2000 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Protected
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
vm.stopPrank();
​
vm.startPrank(user3);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 1000);
vm.stopPrank();
​
vm.startPrank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.stopPrank();
​
vm.startPrank(user3);
address stockAddr1 = GenerateAddress.generateStockAddress(1);
preMarktes.abortBidTaker(stockAddr1, offerAddr);
vm.stopPrank();
​
uint256 user3Balance =
tokenManager.userTokenBalanceMap(user4, address(mockUSDCToken), TokenBalanceType.SalesRevenue);
​
// fails
assert(user3Balance > 0);
}

Impact

As a result of this incorrect calculation, the Taker is unable to retrieve the correct amount of their deposited funds. In many cases, the calculation results in the Taker receiving 0 tokens, effectively causing the deposited funds to be stuck in the contract without any way for the Taker to recover them.

Precision loss amplifies the financial discrepancy, leading to a situation where even if the Taker is supposed to receive a small amount of tokens, the amount is rounded down to 0. This not only prevents the Taker from recovering their funds but also leads to a loss of confidence in the contract’s ability to handle financial transactions accurately.

The inability to retrieve funds not only causes financial harm to the Taker but also undermines the overall trust and functionality of the contract. Users may be reluctant to engage with the contract if they believe their funds could become inaccessible.

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);
}
​
// Audit wrong calculation
- 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, 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());
}
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.