Tadle
Tadle
DeFiFoundry
27,750 USDC
View results
Previous Next
Submission Details
Severity: medium
Valid

Inability to Use Wrapped Native Token (WETH)

cryptomoon

Summary

The current implementation in the token transfer logic(tillIn function in TokenManager contract) does not allow users to use Wrapped Ether (WETH) when tokenAddress is equal to wrappedNativeToken. Instead, the contract expects the user to send Ether (ETH) directly. This limitation forces users to convert WETH back to ETH before making transactions, which can be cumbersome and inconvenient, particularly in scenarios where users prefer to work with WETH.

Vulnerability Details

The contract logic requires users to convert their WETH to ETH when tokenAddress is equal to wrappedNativeToken. If the user holds WETH and attempts to send it directly, the contract will not accept it, as it expects ETH instead of WETH. Users may be confused by the need to convert WETH to ETH, especially if they are not familiar with the difference between the two. This could result in failed transactions, as users might not understand why their WETH was rejected by the contract.

As it can be seen in below snippet, if _tokenAddressis wrappedNativeToken, the contract assumes that equivalent amount of native tokensneeds to be transferred.

https://github.com/Cyfrin/2024-08-tadle/blob/main/src/core/TokenManager.sol#L79-L99

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 {
/// @notice token is ERC20 token
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}

Impact

No suppport of wrappedNativeTokenby the protocol.

Tools Used

Manual

Recommendations

The fixed snippet can be:

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 == 0) {
/// @notice token is ERC20 token
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
if (msg.value < _amount) {
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
);
}
Updates

Lead Judging Commences

0xnevi Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

finding-TokenManager-tillin-wrapper-inconsistent

Valid medium severity, given it is noted in contest READ.ME that any standard ERC20 tokens should be supported. Although arguably could be low severity, given users can simply unwrap WETH to native ETH and perform the deposits via `tillIn()`, I will leave open for discussions, but taking READ.ME as the source of truth, I believe medium severity is appropriate, given it is explicitly noted that this token should be compatible#9##. The fix would be to utilize a zero address or equivalent to represent native ETH when wrapping to WETH. > Tokens: - ETH - WETH - ERC20 (any token that follows the ERC20 standard)

Support

FAQs

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