Skip to content
This repository has been archived by the owner on Mar 3, 2024. It is now read-only.

KingNFT - A previously removed LMPVault can never been added to LMPVaultRegistry again #504

Closed
sherlock-admin opened this issue Aug 30, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Aug 30, 2023

KingNFT

medium

A previously removed LMPVault can never been added to LMPVaultRegistry again

Summary

The removeVault() function of LMPVaultRegistry contract is missing to remove vaultAddress from _vaultsByType mapping, attempting to add a previously removed LMPVault would always revert.

Vulnerability Detail

Please look at the implementation of removeVault(), vaultAddress is removed from _vaultsByAsset mapping, but not removed from _vaultsByType mapping. Hence, addVault() with any previously removed vaultAddress would revert on L59.

File: src\vault\LMPVaultRegistry.sol
46:     function addVault(address vaultAddress) external onlyUpdater {
47:         Errors.verifyNotZero(vaultAddress, "vaultAddress");
48: 
49:         ILMPVault vault = ILMPVault(vaultAddress);
50: 
51:         address asset = vault.asset();
52:         bytes32 vaultType = vault.vaultType();
53: 
54:         if (!_vaults.add(vaultAddress)) revert VaultAlreadyExists(vaultAddress);
55:         //slither-disable-next-line unused-return
56:         if (!_assets.contains(asset)) _assets.add(asset);
57: 
58:         if (!_vaultsByAsset[asset].add(vaultAddress)) revert VaultAlreadyExists(vaultAddress);
59:         if (!_vaultsByType[vaultType].add(vaultAddress)) revert VaultAlreadyExists(vaultAddress);
60: 
61:         emit VaultAdded(asset, vaultAddress);
62:     }
63: 
64:     function removeVault(address vaultAddress) external onlyUpdater {
65:         Errors.verifyNotZero(vaultAddress, "vaultAddress");
66: 
67:         // remove from vaults list
68:         if (!_vaults.remove(vaultAddress)) revert VaultNotFound(vaultAddress);
69: 
70:         address asset = ILMPVault(vaultAddress).asset();
71: 
72:         // remove from assets list if this was the last vault for that asset
73:         if (_vaultsByAsset[asset].length() == 1) {
74:             //slither-disable-next-line unused-return
75:             _assets.remove(asset);
76:         }
77: 
78:         // remove from vaultsByAsset mapping
79:         if (!_vaultsByAsset[asset].remove(vaultAddress)) revert VaultNotFound(vaultAddress);
80: 
81:         emit VaultRemoved(asset, vaultAddress);
82:     }

Impact

Any previously removed LMPVaults can never been added to LMPVaultRegistry again

Code Snippet

https://github.com/sherlock-audit/2023-06-tokemak/blob/main/v2-core-audit-2023-07-14/src/vault/LMPVaultRegistry.sol#L64

Tool used

Manual Review

Recommendation

Removing LMPVault from _vaultsByType mapping when removeVault().

Duplicate of #674

@github-actions github-actions bot added Medium A valid Medium severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Sep 11, 2023
@sherlock-admin2 sherlock-admin2 changed the title Obedient Sandstone Parrot - A previously removed LMPVault can never been added to LMPVaultRegistry again KingNFT - A previously removed LMPVault can never been added to LMPVaultRegistry again Oct 3, 2023
@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Oct 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label Medium A valid Medium severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

2 participants