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

`Amount-msg.value` Not Returned to caller and Loss of ETH When Calling tillIn with ERC20 Token

aresaudits

Summary

Vulnerability Details

  1. In the TokenManager.sol contract, there is a check to ensure that the msg.value provided by the caller is at least equal to the required _amount. If msg.value is less than _amount, the function reverts with an error. However, the function does not handle the scenario where msg.value is greater than _amount, resulting in the excess amount not being returned to the caller. This can lead to users overpaying and losing funds unintentionally.

  2. he tillIn function handles deposits of either native tokens (ETH) or ERC20 tokens. However, there is a potential issue when the function is called with ETH (i.e., msg.value is greater than 0) but the _tokenAddress corresponds to an ERC20 token. In this scenario, the function does not revert, and the user effectively loses the msg.value sent with the transaction because it is not used or refunded.

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) {//@audit-does not handle `amount-msg.value` case
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
);
}
​
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}

This Issue makes sense because the contract charges multiple fees like Platform,tax etc.due to this user may send more eth to the contract and hence the `amount-msg.value` will be trapped in the contract

Impact

Users may lose funds if they send more Ether than required. This can lead to a poor user experience and potential loss of trust in the contract.

Tools Used

Manual Review

Recommendations

  1. Modify the function to return any excess msg.value to the caller

  2. add a check at the beginning of the function to ensure that msg.value is zero when _tokenAddress is not the wrapped native token

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!