GimelSec - Secondary currencies can be fee-on-transfer tokens

[sherlock-admin]

GimelSec

medium

Secondary currencies can be fee-on-transfer tokens

Summary

Vault disallows transfer fees on assetToken and underlyingToken from borrowCurrencyId, but it doesn't check assetToken and underlyingToken from secondaryBorrowCurrencies.

Vulnerability Detail

The updateSecondaryBorrowCapacity() function checks assetToken and underlyingToken from secondaryCurrencyId that should not have transfer fees.
But in the setVaultConfig() function, it doesn't check assetToken.hasTransferFee and underlyingToken.hasTransferFee from secondaryBorrowCurrencies.

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 and underlyingToken from secondaryBorrowCurrencies in setVaultConfig() function.

[jeffywu]

Fair enough, we should put this check in as well. Since listing those currencies is authenticated, I would consider this to be low severity.

[jeffywu]

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.