Tadle
Tadle
DeFi
30,000 USDC
View results
Previous Next
Submission Details
Severity: low
Invalid

Incorrect NatSpec Comment on `TokenManager::withdraw` Function Regarding Ownership Restriction

pro_king

Description

The NatSpec comment in the TokenManager::withdraw function inaccurately states that "Caller must be owner," implying that only the contract owner can invoke this function. However, the function's implementation allows any user to call the withdraw function to withdraw their own tokens. This mismatch between the NatSpec comment and the actual function logic can cause confusion for developers and auditors, leading to potential misuse or misunderstanding of the function.

/**
* @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);
}

Impact

  1. Audit Complexity: Auditors may need to spend additional time verifying the accuracy of comments versus the implementation, potentially overlooking other critical issues.

  2. Potential Misuse: If a developer incorrectly assumes that only the owner can call the function, they might overlook proper access control or assume certain security measures are in place when they are not.

Tools Used

Manual Review

Recommendations

Correct the NatSpec Comment: Update the NatSpec comment to accurately reflect the function's intended use. Instead of stating "Caller must be owner," it should describe that any user can call this function to withdraw their tokens.

/**
* @notice Withdraw
- * @dev Caller must be owner
+ * @dev Any user can call this function to withdraw their tokens
* @param _tokenAddress Token address
* @param _tokenBalanceType Token balance type
*/
Updates

Lead Judging Commences

0xnevi Lead Judge 10 months 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.