Tadle

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

CreateTaker() funds will be frozen or exceeded by abortBidTaker()

Summary

The deposit funds when createTaker() is called will be frozen by abortBidTaker() in the PreMarkets.sol.

Vulnerability Details

When createTaker() user calls abortBidTaker(), depositAmount of funds will be frozen or exceed in the PreMarkets.sol(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
);

The formula of depositAmount calculation as follow is wrong.

depositAmount = stockInfo.points * preOfferInfo.points / preOfferInfo.amount
Let's analyze the below test code.

function test_abort_turbo_offer() public {
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
vm.stopPrank();
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
preMarktes.createTaker(offerAddr, 500);
vm.stopPrank();
vm.prank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.startPrank(user1);
address stock1Addr = GenerateAddress.generateStockAddress(1);
preMarktes.abortBidTaker(stock1Addr, offerAddr);
vm.stopPrank();
}

After user1 calls abortBidTaker(), user1's userTokenBalanceMap[address][tokenAddress][MakerRefund] is 0. So user1 couldn't withdraw nothing from the contract.

Tools Used

Manual Review

Recommendations

The code in the (https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L671-L675) must be changed as follows.

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.