Tadle

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

Excess ether could not be returned to the user inside the tillIn function of the TokenManager

Summary

Loss of excess ether inside the tillIn() function of the TokenManager contract

Vulnerability Details

The tillIn() function of the TokenManager contract is used to get a user's msg.value and after wrapping it, transfers the token to the pool in the case of wrappedNativeToken is the _tokenAddress. Users could send more ether than the actual purchase price in order to ensure that they can 'till in'. However, the tillIn() function did not return the excess ether, which would cause asset loss to the user. Consider the following scenario:

  1. User A believes that the value of tilling in is 0.3 eth, considering that it may be revert due to price changes, so user A passes 0.305 eth to till in with 0.3 ether

  2. User A's transaction is executed, the 0.3 ether is wrapped and transferred, but since the contract does not return excess eth, user A actually loses 0.005 eth.

Impact

Fund loss due to not returning the excess ETH inside the function tillIn()

Tools Used

Manual

Recommendations

Consider returning the excess ETH when it is more than the amount variable:

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);
+ if (msg.value > _amount) {
+ payable(msg.sender).transfer(msg.value - _amount);
+ }
} else {
/// @notice token is ERC20 token
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
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.