Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

No access control in `core::TokenManager::withdraw` function leading anyone to invoke withdraw function

x0rc1ph3r

Summary

The TokenManager::withdraw function in the smart contract lacks proper access control mechanisms. This vulnerability allows any user to potentially invoke the function, resulting in unauthorized withdrawals of tokens from the contract.

Vulnerability Details

The vulnerability is located in the withdraw function, defined 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];
​
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
);
}

Despite the natspec indicating that the caller must be the owner, there is no explicit check or modifier enforcing this requirement. There is only one modifier checking whenNotPaused. No onlyOwner modifier. This omission allows any caller to execute the function and withdraw tokens.

Impact

The lack of access control in the withdraw function could have severe consequences:

@> Unauthorized users could withdraw tokens, leading to significant financial losses for the protocol.
@> The integrity and security of the contract could be compromised, resulting in a loss of trust from users and investors.
@> The protocol may face potential legal and reputational damages.

Tools Used

  • Manual code review

Recommended Mitigation

To mitigate this vulnerability, it is essential to implement proper access control mechanisms.

Ownership Check:
Add an onlyOwner modifier to the withdraw function to ensure that only the contract owner can call it.

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
- ) external whenNotPaused {
+ ) external whenNotPaused onlyOwner {
uint256 claimAbleAmount = userTokenBalanceMap[_msgSender()][
_tokenAddress
][_tokenBalanceType];
​
if (claimAbleAmount == 0) {
return;
}
.
.
.
emit Withdraw(
_msgSender(),
_tokenAddress,
_tokenBalanceType,
claimAbleAmount
);
}
Updates

Lead Judging Commences

0xnevi Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-TokenManager-withdraw-lack-access-control

Invalid, withdrawals are gated to caller context `msg.sender`, not anybody. This acts as the access control and hence "owner", to withdraw collateral/points tokens after finalization of market actions.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!