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

Reentrancy in TokenManager.sol:withdraw() can drain Capital Pool

oxwhite

Summary

The withdraw function in TokenManager.sol is open to reentrancy attack, which will result in draining the pool.

Vulnerability Details

The withdraw function defined as :

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

The function retrieves claimAbleAmount for the msg.sender and deals with transfer of either native token or ERC20 withdrawals. However, In both cases , whether the token is native or Erc20 token, there is no update regarding userTokenBalanceMap before executing the transfer, which allows the user to reenter the function and potentially drain the pool.

Imagine a sample attack scenario:

  1. An attacker contract(Bob) has some native or Erc20 to claim and calls withdraw().

  2. Bob's claimAbleAmount is correctly retrieved, but his related balance in userTokenBalanceMapis not updated.

  3. Bob repeatedly can reenter the pool and withdraw the same amount until the pool has no funds.

Impact

The pool can be drained.

Tools Used

Manual Review

Recommendations

update the balance of msg.sender before transfering the claimAbleAmount. Add the following line of code

userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;

before cashing the address of capital Pool(capitalPoolAddr)

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.