Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
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

0xmuxyz

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 over 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.

Give us feedback!