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

`call()` should be used instead of `transfer()` on an address payable

0xDazai

Relevant github links

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L169

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/utils/Rescuable.sol#L13-L14

Summary

The transfer() and send() functions forward a fixed amount of 2300 gas. The gas cost of EVM instructions may change significantly during hard forks which may break already deployed contract systems that make fixed assumptions about gas costs. For example. EIP 1884 broke several existing smart contracts due to a cost increase of the SLOAD instruction.

Vulnerability Details

In TokenManager.sol , withdraw() function there it checks if the tokenAddress of the token that will be withdrawn is wrappedNativeToken. If it is it calls the transfer() function with address payable.

function withdraw(
address _tokenAddress,
TokenBalanceType _tokenBalanceType
) external whenNotPaused {
....
....
....
​
if (_tokenAddress == wrappedNativeToken) {
/**
* @dev token is native token
* @dev transfer from capital pool to msg sender
* @dev withdraw native token to token manager contract
* @dev transfer native token to msg sender
*/
_transfer(
wrappedNativeToken,
capitalPoolAddr,
address(this),
claimAbleAmount,
capitalPoolAddr
);
​
​
@>> IWrappedNativeToken(wrappedNativeToken).withdraw(claimAbleAmount);
@>> payable(msg.sender).transfer(claimAbleAmount);
} else {
....
....
);
}
​
​
.....
}

Also in tillIn() function which is used to transfer the msg.sender tokens to the capitalAddress , _safe_transfer() function is used if _tokenAddress == wrappedNativeToken . The _safe_transfer() is part of the Rescuable.sol contract which is not in scope, but it has the same vulnerability because it uses the transfer method in form of TRANSFER_SELECTOR being called.

function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
....
....
....
​
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 {
....
);
}
​
....
}
function _safe_transfer(
address token,
address to,
uint256 amount
) internal {
(bool success, ) = token.call(
abi.encodeWithSelector(TRANSFER_SELECTOR, to, amount)
);
​
​
if (!success) {
revert TransferFailed();
}
}

Impact

The transfer() function only allows the recipient to use 2300 gas. If the recipient uses more than that, transfers will fail. In the future gas costs might change increasing the likelihood of that happening.

Possible cases :
The claimer smart contract does not implement a payable function; does implement a payable fallback which uses more than 2300 gas unit or implements a payable fallback function that needs less than 2300 gas units but is called through proxy; raising the call's gas usage above 2300;
Additionally, using higher than 2300 gas might be mandatory for some multisig wallets.

Tools Used

Manual Review

Recommendations

Use call() instead of transfer() to transfer native tokens.

Updates

Lead Judging Commences

0xnevi Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

[invalid] finding-TokenManager-withdraw-transfer-2300-gas

Invalid, known issues [Medium-2](https://github.com/Cyfrin/2024-08-tadle/issues/1)

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.