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

User can drain all funds from `TokenManager`.

baz1ka

Relevant GitHub Links

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

Vulnerability Details

TokenManager.withdraw() allows user to withdraw their funds from the protocol if they have any tokens:

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
);
}

userTokenBalanceMap is a mapping that stores all users' tokens that are available for withdrawal. If there are available tokens function will call the transfer method to transfer tokens from CapitalPool to the user. Neither before nor after the transfer there is no reset functionality that sets userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] to 0 which allows user to call this function again and again until there is not tokens left in CapitalPool.

Impact

Any user can steal funds from other users.

Recommended Mitigation Steps

Change TokenManager.withdraw() to also reset user balance after withdrawal:

RelatedContractLibraries.CAPITAL_POOL
);
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
+
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
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.