Tadle

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

Due to an insufficient validation, the difference of the amount of native ETH (`msg.value - _amount`) will not be deposited and therefore it will be stuck forever inside the TokenManager contract

Summary

Due to an insufficient validation, the difference of the amount of native ETH (msg.value - _amount) will not be deposited and therefore it will be stuck forever inside the TokenManager contract - when the TokenManager#tillIn() would be called.

Vulnerability Details

Within the following functions, the TokenManager#tillIn() to deposit WETH into the CapitalPool contract would be called:

Also, the TokenManager#tillIn() can directly be called to deposit WETH into the CapitalPool contract.

Within the TokenManager#tillIn(), if a given _tokenAddress would be the wrappedNativeToken (_tokenAddress == wrappedNativeToken), the following three steps would be proceeded:

/**
* @notice Till in, Transfer token from msg sender to capital pool
* @param _accountAddress Account address
* @param _tokenAddress Token address
* @param _amount Transfer amount
* @param _isPointToken The transfer token is pointToken
* @notice Capital pool should be deployed
* @dev Support ERC20 token and native token
*/
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 (_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) { ///<---- @audit - Only check whether or not the "msg.value" is less than a given "_amount"
revert Errors.NotEnoughMsgValue(msg.value, _amount); ///<---- @audit
}
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}(); ///<---- @audit - WETH# deposit()
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount); ///<---- audit - Transfer WETH to the Capital Pool

At the step 1/ above in the TokenManager#tillIn(), the msg.value is supposed to be equal to a given _amount (msg.value == _amount).

However, at the step 1/ above, whether or not the msg.value is less than a given _amount (msg.value < _amount) would only be validated like this:
https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L86

if (msg.value < _amount) { ///<---- @audit - Only check whether or not the "msg.value" is less than a given "_amount"
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}

This is problematic because, if the msg.value is larger than a given _amount (msg.value > _amount), the difference of the amount of native ETH (msg.value - _amount) will be stuck forever inside the TokenManager contract.
(NOTE:To be exact, in this case, only a given _amount of native ETH would be deposited into the WETH contract to convert to WETH via the IWrappedNativeToken(wrappedNativeToken)#deposit(). However, the difference of the amount of native ETH (msg.value - _amount) will not be deposited and therefore it will remain in the TokenManager contract)

Impact

Within the TokenManager#tillIn(), if _tokenAddress == wrappedNativeToken and the msg.value is larger than a given _amount (msg.value > _amount), the difference of the amount of native ETH (msg.value - _amount) will be stuck forever inside the TokenManager contract.

Tools Used

  • Foundry

Recommendations

Within the TokenManager#tillIn(), consider adding a condition, which check whether or not the msg.value is larger than a given _amount (msg.value > _amount) like this:

+ if (msg.value < _amount || msg.value > _amount) {
- if (msg.value < _amount) {
revert Errors.NotEnoughMsgValue(msg.value, _amount);
}
Updates

Lead Judging Commences

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