User will Lose Ether when sending msg.value to vault with tokenType not being Ether
Summary
It is possible for a user to accidently send ETH when executing VaultAccountAction.sol'senterVault() and rollVaultPosition() even though the vault's tokenType is not Ether.
When the vault's tokenType is not Ether, there is no meaning of sending ETH so user can lose their ETH accidentally.
Vulnerability Detail
VaultAccountAction.sol'senterVault() and rollVaultPosition() is payable but when the vault's tokenType is not Ether,
there is no meaning for user to send ETH. However there is no check for msg.value when tokenType is NOT Ether, so it is possible
for user to send ETH accidentally and losing their fund.
TomJ
medium
User will Lose Ether when sending
msg.value
to vault withtokenType
not being EtherSummary
It is possible for a user to accidently send ETH when executing
VaultAccountAction.sol's
enterVault()
androllVaultPosition()
even though the vault'stokenType
is not Ether. When the vault'stokenType
is not Ether, there is no meaning of sending ETH so user can lose their ETH accidentally.Vulnerability Detail
VaultAccountAction.sol's
enterVault()
androllVaultPosition()
is payable but when the vault's tokenType is not Ether, there is no meaning for user to send ETH. However there is no check for msg.value when tokenType is NOT Ether, so it is possible for user to send ETH accidentally and losing their fund.Impact
User will lose their Ether.
Code Snippet
User can send ETH since
enterVault()
androllVaultPosition()
is payable https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L42 https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L96 https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L177For
enterVault()
function, it will do an external call tovaultConfig.transferUnderlyingToVaultDirect
where there is no check of msg.value when tokenType is NOT Ether. https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L66 https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/internal/vaults/VaultConfiguration.sol#L457-L460For
rollVaultPosition()
function, it will do an external call tovaultAccount.depositForRollPosition
where there is not check of msg.value when tokenType is NOT Ether. https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/external/actions/VaultAccountAction.sol#L138 https://github.com/notional-finance/contracts-v2/blob/cf05d8e3e4e4feb0b0cef2c3f188c91cdaac38e0/contracts/internal/vaults/VaultAccount.sol#L271-L275Tool used
Manual Review
Recommendation
Add check of whether
msg.value
is zero or not whentokenType
is NOT Ether