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

`userTokenBalanceMap` is never set 0 after withdrawal allowing users to steal funds

x18a6

Summary

userTokenBalanceMap is never set 0 after withdrawal allowing users to steal funds.

Vulnerability Details

TokenManager::withdraw(...) function checks the claimAbleAmount for the user and if it's greater than zero, it allows the user to withdraw that amount. However, after the withdrawal, the claimAbleAmount is not reset to zero. This means that if the withdraw function is called again, the user could withdraw the same amount again, even though they've already claimed their tokens.

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
*/
// @audit this automatocally approves for msg.sender
_safe_transfer_from(_tokenAddress, capitalPoolAddr, _msgSender(), claimAbleAmount);
// _safe_transfer_from(_tokenAddress, , _msgSender(), claimAbleAmount);
}
​
//@audit missing reset for userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType]
emit Withdraw(_msgSender(), _tokenAddress, _tokenBalanceType, claimAbleAmount);
}

Let's consider an example where Bob has 100 tokens in his refund:

  • Bob calls the withdraw function to withdraw all his 100 tokens. The claimAbleAmount for Bob is now 100 tokens.

  • The withdraw function checks if claimAbleAmount is greater than zero, which it is, so it allows the withdrawal.

  • Bob successfully withdraws 100 tokens. However, the claimAbleAmount is not reset to zero after the withdrawal.

  • Now, Bob calls the withdraw function again. Even though he has already withdrawn all his tokens, the claimAbleAmount is still 100.

  • The withdraw function again checks if claimAbleAmount is greater than zero, which it is, so it allows another withdrawal.

Bob is now able to withdraw another 100 tokens, even though he should only have been able to withdraw 100 in total. He has effectively doubled his withdrawal due to the bug.

Impact

Steal of funds.

Tools Used

Manual review.

Recommendations

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) {
...
} else {
...
}
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
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.