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

Unable To `TokenMananger::tillIn` Using `wrappedNativeToken`

auditweiler

Summary

The implementation of tillIn(address,address,uint256,bool) forces the msgSender() to always transact using msg.valuewhen attempting to use a tokenAddress of wrappedNativeToken, they cannot use the ERC-20 itself.

Vulnerability Details

Invocations to tillIn(address,address,uint256,bool) have special handling for wrappedNativeToken; it unconditionally deposits call's msg.value into the IWrappedNativeToken (i.e. WETH):

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

Consequently, this makes it impossible to transact using purely the IWrappedNativeToken token balance of the caller via the _transfer(address,address,address,uint256,address), since this takes place in the opposing conitional branch designed specifically for ERC-20 transfers.

They have to use msg.value, no exceptions.

Impact

Limited composability and arguable dependent client DoS for a crucial token type, since it is impossible to tillIn(address,address,uint256,bool) using a raw IWrappedNativeToken ERC-20 balances; callers are forced to transact using msg.value exclusively.

This will no doubt attenuate the effectiveness of the TokenManager.

Tools Used

Manual Review

Recommendations

When transacting using wrappedNativeToken, the amount supplied to the call should represent the IWrappedNativeToken balance, which if non-zero, should be pulled from the caller.

If a non-zero msg.valueis supplied, we must then enforce that the msg.senderhas additionally declared a _tokenAddressof IWrappedNativeToken, else revert.

Finally, the non-zero msg.valueshould be deposited to the IWrappedNativeToken contract, and the amount should be increased to reflect this additional deposited amount.

/**
* @notice Till in, Transfer token from msg sender to capital pool
* @param _accountAddress Account address
* @param _tokenAddress Token address
* @param _amount Transfer amount
* @param _isPointToken The transfer token is pointToken
* @notice Capital pool should be deployed
* @dev Support ERC20 token and native token
*/
function tillIn(
address _accountAddress,
address _tokenAddress,
uint256 _amount,
bool _isPointToken
)
external
payable
onlyRelatedContracts(tadleFactory, _msgSender())
onlyInTokenWhiteList(_isPointToken, _tokenAddress)
{
address capitalPoolAddr = tadleFactory.relatedContracts(
RelatedContractLibraries.CAPITAL_POOL
);
if (capitalPoolAddr == address(0x0)) {
revert Errors.ContractIsNotDeployed();
}
​
/// @notice If the caller is trying to deposit ERC-20 tokens:
if (_amount != 0) {
/// @notice token is ERC20 token, transfer it to the address
_transfer(
_tokenAddress,
_accountAddress,
capitalPoolAddr,
_amount,
capitalPoolAddr
);
}
​
/// @notice If we're handling native ether...
if (msg.value != 0) {
/// @notice Ensure the caller is trying to transact
/// @notice using `wrappedNativeToken`, since we will
/// @notice deposit this on the user's behalf and attempt
/// @notice to append this to the specified token balance.
require(_tokenAddress == wrappedNativeToken);
​
/// @notice deposit the wrapped tokens
IWrappedNativeToken(wrappedNativeToken).deposit{value: _amount}();
_safe_transfer(wrappedNativeToken, capitalPoolAddr, _amount);
​
/// @notice increase the `_amount` to reflect the user's updated deposit
_amount += msg.value;
}
​
// @notice return if amount is 0
if (_amount == 0) {
return;
}
emit TillIn(_accountAddress, _tokenAddress, _amount, _isPointToken);
}
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.