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

Users can drain all the capitalPoolAddr because the Balance not cleared/updated after users withdraw.

bigsam

Summary

The vulnerability allows users to repeatedly withdraw funds without updating their balance, posing a significant risk of draining the entire pool.

Vulnerability Details

The vulnerability lies in the `withdraw` function, where the

  1. user's claimable amount is retrieved using the following mapping:

/**
* @notice Withdraw
* @dev Caller must be owner
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
@audit>> Balancce not cleared after it was check >> uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
@audit>>NO Update Re-entrancy and pool draining possible >>
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
);
}
​
/**

2. However, this mapping is not updated to zero after the user withdraws their funds.

3. As a result, the user's balance remains unchanged, leading to a potential re-entrancy attack for ETH withdrawals and continuous claiming of the same amount for ERC20 tokens.

4. This loophole allows a malicious user to drain the entire capital pool.

Impact

A single user could repeatedly withdraw funds, effectively draining the pool.

Tools Used

- Manual code review

Recommendations

To mitigate this vulnerability, it is recommended to set the user's balance to zero immediately after retrieving their claimable amount. This can be done by updating the mapping as follows:

​
/**
* @notice Withdraw
* @dev Caller must be owner
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
++ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
​
if (claimAbleAmount == 0) {
return;
}

This ensures that once a user withdraws their funds, their balance is correctly updated, preventing any re-entrancy attacks or multiple claims on the same balance.

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.