The Standard

Summary

• Large part of the system rely on fetching data from [SmartVaultIndex](https://github.com/the-standard/smart-vault/blob/6517e1da5fe311c27b603ec17d26576f54dc43ed/contracts/SmartVaultIndex.sol#L7) contract and the system as a whole relay on each other contract to function well

• There is a function inside SmartVaultIndex, which is OOS for this audit, but can result in major bad impact on contracts in scope.

• The problem is when the VaultManager is changed inside SmartVaultIndex with new manager. The new one would override all previous SmartVault addresses

Vulnerability Details

• It is a valid case for the team to want to deploy new manager with new terms, or to reduce gas costs, if there are many

• Also there is a valid and ready to use function for such cases:

• But if this happens the new manager would be deployed with lastToken set to 0, which means that each mint will call addVaultAddress with tokenId which is already there and functioning. This could happen for all vaults, which have been minted from the old manager.

Here is the mappings, which are gonna be messed:

PoC

In the following gist I have provided coded PoC and instructions how to run it

Impact

• Bad debt generation, because it is impossible to find the correct vault

• Blocked functionality to transfer ownership of previous vaults, because transaction will revert here

Tools Used

Manual Review
Hardhat

Recommendations

Make manager inside SmartVaultIndex immutable and deploy new index per manager, so this scenario is impossible to be reached: