Summary

A malicious user can deny any user of the protocol the ability to transfer vault tokens in SmartVaultManagerV5.sol.

Vulnerability Details

A malicious user can pick any target user, suppose the attacker is Bob and the user is Alice. Bob can follow this call sequence multiple times

1. Bob mints a new vault

2. Bob transfers the new vault to Alice.

Bob can repeat this as many times as necessary so that once Alice calls the transferFrom() function in the base ERC721 implementation the
_afterTokenTransfer() hook in SmartVaultManagerV5.sol fails due to exceeding the block gas limit.

This is possible because the line

smartVaultIndex.transferTokenId(_from, _to, _tokenId);

calls the function transferTokenId() in the smart vault Index, which in turn calls the removeTokenId() in SmartVaultIndex.sol.

This function will revert due to an unbounded array namely

uint256[] memory currentIds = tokenIds[_user];

the tokenIds Array which Bob has inflated.

The root cause of the token transfer will be the loop

for (uint256 i = 0; i < idsLength; i++) {
if (currentIds[i] != _tokenId) tokenIds[_user].push(currentIds[i]);
}
The greater the number of vault tokens a user has the closer to exceeding the block gas limit. Since there is no limit to prevent Bob from transferring numerous vaults to Alice this attack is viable.

Impact

This denies a user of the protocol the ability to transfer vaults from their account.

Tools Used

Manual Review

Recommendations

I would recommend a limit on the number of vaults a user can have.