Lines of code

Vulnerability details

C4 issues

M-10: Users's tokens stuck in AutoCompound after Vault is deactivated

Comments

The issue comes from the fact that vault can be deactivated in AutoCompound, while function withdrawLeftoverBalances requires that the caller is either the UniswapV3 position owner, or the user has deposited their NFT position to an active vault:

function withdrawLeftoverBalances(uint256 tokenId, address to) external nonReentrant {
        address owner = nonfungiblePositionManager.ownerOf(tokenId);
        if (vaults[owner]) {
            owner = IVault(owner).ownerOf(tokenId);
        }
        if (owner != msg.sender) {
            revert Unauthorized();
        }
}

If a vault is deactivated, then user cannot withdraw because they are not the position owner (because they already deposited NFT to the vault) and the vault is deactivated -> vaults[owner] = false

Mitigation

PR #18, PR #29

In PR #18, function setVault is now only used for activating vault, deactivating vault is not possible anymore:

    function setVault(address _vault) external onlyOwner {
        emit VaultSet(_vault);
        vaults[_vault] = true;
    }

This function then moved to file Transformer.sol in PR #29 and AutoCompound now inherits Transformer. The mitigation resolved the original issue.

Conclusion

LGTM

code-423n4 / 2024-04-revert-mitigation-findings