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

Wrong refund amount in abortBidTaker leads to loss of funds for the taker

ge6a

Summary

PreMarkets.abortBitTaker is a function used by the taker to get a refund of the collateral they deposited for purchasing a certain amount of points in case the corresponding offer is aborted. The problem is that the calculation is not performed correctly, resulting in the taker aborting their bid but not receiving back the initially locked collateral.

Vulnerability Details

Let's first look at how the collateral to be deposited when creating a taker is calculated through createTaker.

uint256 depositAmount = _points.mulDiv(
offerInfo.amount,
offerInfo.points,
Math.Rounding.Ceil
);

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L212-L216

The points the taker wants to purchase are multiplied by the total amount for the entire offer and divided by the total number of points for that offer. The goal is to obtain the portion of the amount that is proportional to the points being purchased when creating this taker. Now, let's examine how the amount of tokens to be refunded to the taker is calculated.

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

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L671-L675

It can be seen that the points in the corresponding stock are multiplied by the points in the offer and divided by the amount. This is almost always 0, and even in cases where it is not 0, the value is quite different from what was paid. As a result, the taker loses their refund. It can be seen in the POC below.

POC

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();
​
console.log("Refund balance %d", tokenManager.userTokenBalanceMap(
address(user1),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
)); // it is 0
}

Impact

Loss of funds for the taker.

Tools Used

Manual review

Recommendations

Fix the calculation of the deposit amount in abortBidTaker to be same as the one in createTaker.

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.