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

Anyone can drain the capital pool by calling withdraw multiple times because the userTokenBalanceMap is not deducted even after someone withdraws their balance.

touthang

Summary

The withdraw function does not deduct the balance of the withdrawer which makes it possible for any user that has some amount of balance to drain the capital pool.

Vulnerability Details

As we can see in the function withdraw, we get the amount to withdraw by uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType]; but never actually reset the userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType].

Example-

  • An attacker(acting as a maker) creates an offer and closes it just to increase his MakerRefund balance.

  • Now attacker can call withdraw unlimited amount of times to drain the capital pool.

Impact

draining of funds

Tools Used

manual

Recommendations

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
+ userTokenBalanceMap[_msgSender()][_tokenAddress][_tokenBalanceType] = 0;
​
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
);
}

Make sure to add the line before any transfer is done because the next if statement will transfer eth directly to the user which makes it possible to reenter. So adding that line above before any transfer is made is crucial.

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.