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

capitalPool can be drained due to how the withdraw function is implmented

inzinko

Summary

The withdraw function is implemented incorrectly, and it allows two types of issue to occur

  • Rentrancy becuase the function as no nonrentrant modifier nor does it reset the claimableAmount to zero before performing the external call to the user making it vulnerable to the exploit

  • Becuase it does not reset the claimble amount for that particular _tokenBalanceType users can just keep withdrawing until they drain the capitalPool.

Vulnerability Details

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

As shown in the snippet above no reentrancy modifier and no update of claimable amount which allows user to both perform a reentrancy attack or keep withdrawing until the protocol is drained

Impact

The capitalPool is the backbone of the protocol, that holds all funds that users trade with and collateral, and also the platform fees itself, draining it will make the protocol insolvent and cause loss of funds for both users and the protocol

Tools Used

Manual Review

Recommendations

  • implement a nonRentrant modifier on the function from openZepplin libraries and also additional security to reset the claimable amount for that tokenAddress to zero, since it withdrawals all amount automatically, this will also prevent the infinite withdrawal bug.

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.