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

Extra ETH sent when calling `tillIn` function can be locked in token manager

rbserver

Summary

When extra ETH are sent while calling the tillIn function, such extra ETH can be locked in the token manager unless the token manager's owner calls the rescue function to transfer such extra ETH out from the token manager. However, there is no guarantee that such owner would perform such action.

Vulnerability Details

When calling the tillIn function below with the _tokenAddress input being wrappedNativeToken, if msg.value is more than _amount, the extra ETH that equals msg.value minus _amount is not used so such extra ETH would remain in the token manager.

Similarly, when the _tokenAddress input is not wrappedNativeToken, the tillIn function would not use msg.value in any way. Thus, if some ETH are sent when calling the tillIn function with the _tokenAddress input not being wrappedNativeToken, such sent ETH would remain in the token manager.

https://github.com/Cyfrin/2024-08-tadle/blob/c249cdb68c37c47025cdc4c4782c8ee3f20a5b98/src/core/TokenManager.sol#L56-L103

function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
/// @notice return if amount is 0
if (_amount == 0) {
return;
}
​
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (capitalPoolAddr == address(0x0)) {
revert Errors.ContractIsNotDeployed();
}
​
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @notice check msg value
* @dev if msg value is less than _amount, revert
* @dev wrap native token and transfer to capital pool
*/
@> if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
} else {
/// @notice token is ERC20 token
@> _transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
...
}

Impact

The extra ETH sent to the token manager would be locked in the token manager unless the token manager's owner calls the rescue function to transfer such extra ETH out from the token manager. Yet, there is no guarantee that the token manager's owner would perform such action.

Tools Used

Manual Review

Recommended Mitigation

The tillIn function can be updated to revert if msg.value != _amount is true when the _tokenAddress input is wrappedNativeToken and if msg.value is bigger than 0 when the _tokenAddress input is not wrappedNativeToken.

Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-TokenManager-tillin-excess

Invalid, these are by default, invalid based on codehawks [general guidelines](https://docs.codehawks.com/hawks-auditors/how-to-determine-a-finding-validity#findings-that-may-be-invalid). The check implemented is simply a sufficiency check, it is users responsibility to only send an appropriate amount of native tokens where amount == msg.value when native token is intended to be used as collateral (which will subsequently be deposited as wrapped token). All excess ETH can be rescued using the `Rescuable.sol` contract. > Users sending ETH/native tokens > If contracts allow users to send tokens acc111identally.

Support

FAQs

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