Closed sherlock-admin closed 1 year ago
Fair enough, we should put this check in as well. Since listing those currencies is authenticated, I would consider this to be low severity.
Actually, secondary borrow currencies cannot have their capacities set if there is a transfer fee. This effectively restricts their ability to be used: https://github.com/notional-finance/contracts-v2/blob/01eeec9c64037d614eb621b9f3e9c4ab93388d84/contracts/external/actions/VaultAction.sol#L67-L68
I would consider this issue invalid as a result.
GimelSec
medium
Secondary currencies can be fee-on-transfer tokens
Summary
Vault disallows transfer fees on
assetToken
andunderlyingToken
fromborrowCurrencyId
, but it doesn't checkassetToken
andunderlyingToken
fromsecondaryBorrowCurrencies
.Vulnerability Detail
The
updateSecondaryBorrowCapacity()
function checksassetToken
andunderlyingToken
fromsecondaryCurrencyId
that should not have transfer fees. But in thesetVaultConfig()
function, it doesn't checkassetToken.hasTransferFee
andunderlyingToken.hasTransferFee
fromsecondaryBorrowCurrencies
.Impact
The protocol may allow fee-on-transfer tokens in
secondaryBorrowCurrencies
.Code Snippet
https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/external/actions/VaultAction.sol#L67 https://github.com/sherlock-audit/2022-09-notional/blob/main/contracts-v2/contracts/internal/vaults/VaultConfiguration.sol#L163-L217
Tool used
Manual Review
Recommendation
Check
assetToken
andunderlyingToken
fromsecondaryBorrowCurrencies
insetVaultConfig()
function.