M-10: Users's tokens stuck in AutoCompound after Vault is deactivated
Comments
AutoCompound implements a vaults mapping that lists accepted vaults that can interact with the AutoCompound contract. When AutoCompound.withdrawLeftoverBalances() is called, this mapping is used as a vault whitelist. Therefore, if a user ever deposits their NFT into a vault and the AutoCompound blacklists the vault, the original NFT owner will not be able to withdraw any balances from the AutoCompound contract. Calling AutoCompound.withdrawLeftoverBalances() will lead to a revert.
Mitigation
PR #18
Before discussing the specific mitigation, one change should be mentioned. The protocol has now added a Transformer contract. This contract has the setVault() function. In addition, the AutoCompound contract now inherits this Transformer contract in order to set vault addresses.
The mitigation includes one major change: whitelisting a vault is now irreversible. Whenever Transfer.setVault() is called, the vault can only be whitelisted. This is due to this line of code in setVault(): vaults[_vault] = true;. This ensures an admin can't remove a vault from the whitelist and block specific users/vaults from access to their withdraw balances.
Finally, additional minor changes were made to the VaultSet event which no longer emits if the vault is whitelisted or delisted.
Lines of code
Vulnerability details
C4 issue
M-10: Users's tokens stuck in AutoCompound after Vault is deactivated
Comments
AutoCompound implements a
vaults
mapping that lists accepted vaults that can interact with the AutoCompound contract. When AutoCompound.withdrawLeftoverBalances() is called, this mapping is used as a vault whitelist. Therefore, if a user ever deposits their NFT into a vault and the AutoCompound blacklists the vault, the original NFT owner will not be able to withdraw any balances from the AutoCompound contract. Calling AutoCompound.withdrawLeftoverBalances() will lead to a revert.Mitigation
PR #18
Before discussing the specific mitigation, one change should be mentioned. The protocol has now added a Transformer contract. This contract has the setVault() function. In addition, the AutoCompound contract now inherits this Transformer contract in order to set vault addresses.
The mitigation includes one major change: whitelisting a vault is now irreversible. Whenever Transfer.setVault() is called, the vault can only be whitelisted. This is due to this line of code in setVault():
vaults[_vault] = true;
. This ensures an admin can't remove a vault from the whitelist and block specific users/vaults from access to their withdraw balances.Finally, additional minor changes were made to the VaultSet event which no longer emits if the vault is whitelisted or delisted.
Conclusion
LGTM