Tadle

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

Any user in `userTokenBalanceMap` can drain the protocol

Summary

A critical vulnerability was identified in the TokenManager.sol smart contract that allows any user to withdraw funds multiple times without decrementing their balance. This flaw could enable malicious users to drain the entire capital pool, posing a significant risk to the protocol.

Vulnerability Details

The function addTokenBalance is designed to increase a user's balance in the userTokenBalanceMap. When a user initiates a withdrawal by calling the withdraw() function, the contract checks the user's balance and transfers the corresponding funds.

However, the vulnerability arises from the fact that the user's balance in userTokenBalanceMap is not decremented after a withdrawal. This oversight allows users to repeatedly call the withdraw() function, withdrawing more funds than they initially deposited. As a result, a malicious user could exploit this flaw to drain all the funds from the capital pool.

Code Snippet

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
if (claimAbleAmount == 0) {
return;
}
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
payable(msg.sender).transfer(claimAbleAmount);
} else {
/**
* @dev token is ERC20 token
* @dev transfer from capital pool to msg sender
*/
_safe_transfer_from(
_tokenAddress,
capitalPoolAddr,
_msgSender(),
claimAbleAmount
);
}
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}

Impact

Any user listed in the userTokenBalanceMap could potentially drain all funds from the capital pool.

Proof Of Concept

  1. Bob has previously deposited funds into the protocol, and his balance is tracked in userTokenBalanceMap. For the purposes of this PoC, let's assume Bob's balance is 100 tokens

  2. The current balance of the capital pool is 1,000 tokens

  3. Bob begins by calling the withdraw() function with an amount equal to his balance (100 tokens). Since the withdraw() function does not decrease Bob's balance in userTokenBalanceMap, his balance remains 100 tokens after each withdrawal.

  4. Bob continues to call the withdraw() function in a loop, each time withdrawing 100 tokens. After 10 calls, Bob has successfully withdrawn 1,000 tokens, completely draining the capital pool.

Tools Used

Manual review

Recommendations

Implement logic in withdrawal() that decrements the user's balance in the userTokenBalanceMap after a withdrawal.

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.