Summary
The tillIn function in TokenManager.sol allows the function to continue executing even if msg.value is greater than the specified _amount. However, it only processes the required _amount of ETH and leaves any excess ETH stuck in the contract. This can lead to potential losses for users who inadvertently send more ETH than required, as the surplus ETH becomes trapped and cannot be retrieved.
function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
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 {
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
POC
function test_excessEthNotRefundedDuringCreateOffer() public {
uint256 amountToTillIn = 1 ether;
uint256 excessAmount = 2 ether;
deal(user, excessAmount);
vm.startPrank(user);
uint256 userInitialEthBalance = user.balance;
console2.log("Initial User ETH Balance:", userInitialEthBalance);
uint256 initialWrappedTokenBalance = weth9.balanceOf(address(tokenManager));
console2.log("Initial TokenManager Wrapped Token Balance:", initialWrappedTokenBalance);
weth9.approve(address(tokenManager), type(uint256).max);
CreateOfferParams memory params = CreateOfferParams(
marketPlace,
address(weth9),
1000,
amountToTillIn,
11000,
350,
OfferType.Ask,
OfferSettleType.Protected
);
preMarktes.createOffer{value: excessAmount}(params);
uint256 userIntermediateEthBalance = user.balance;
uint256 intermediateWrappedTokenBalance = weth9.balanceOf(address(tokenManager));
uint256 intermediateEthBalance = address(tokenManager).balance;
console2.log("User ETH Balance after createOffer call:", userIntermediateEthBalance);
console2.log("Intermediate TokenManager Wrapped Token Balance:", intermediateWrappedTokenBalance);
console2.log("Intermediate ETH Balance of TokenManager:", intermediateEthBalance);
uint256 userFinalEthBalance = user.balance;
console2.log("Final User ETH Balance:", userFinalEthBalance);
uint256 finalWrappedTokenBalance = weth9.balanceOf(address(tokenManager));
console2.log("Final TokenManager Wrapped Token Balance:", finalWrappedTokenBalance);
uint256 tokenManagerEthBalance = address(tokenManager).balance;
console2.log("TokenManager ETH Balance:", tokenManagerEthBalance);
assertEq(userFinalEthBalance, userInitialEthBalance - excessAmount, "Excess ETH was not refunded.");
assertEq(tokenManagerEthBalance, 0, "TokenManager has stuck ETH.");
vm.stopPrank();
}
Log
Initial User ETH Balance: 2000000000000000000
Initial TokenManager Wrapped Token Balance: 0
User ETH Balance after createOffer call: 0
Intermediate TokenManager Wrapped Token Balance: 0
Intermediate ETH Balance of TokenManager: 900000000000000000
Final User ETH Balance: 0
Final TokenManager Wrapped Token Balance: 0
TokenManager ETH Balance: 900000000000000000
Error: TokenManager has stuck ETH.
Error: a == b not satisfied [uint]
Expected: 0
Actual: 900000000000000000
Impact
The current implementation of the tillIn function can lead to users inadvertently losing funds. When users send more ETH (msg.value) than the required _amount, the contract only processes the specified _amount and leaves the excess ETH stuck in the contract. Users may not be aware that their extra funds are trapped, leading to unnoticed and potentially significant losses, particularly if large amounts of excess ETH are sent.
Tools Used
Foundry
Recommendations
To prevent users from losing funds due to sending excess ETH, it is advised to modify the tillIn function to only accept the exact required amount of ETH. This can be done by changing the comparison condition from msg.value > _amount to msg.value != _amount in the code. This ensures that the transaction reverts if the ETH sent by the user is not exactly equal to the required _amount, thereby preventing any excess ETH from being sent and becoming stuck in the contract.
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) {
+ if (msg.value != _amount) {
+ // Ensure exact amount of ETH is sent
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);
}
