sherlock-audit / 2024-05-beefy-cowcentrated-liquidity-manager-judging

5 stars 5 forks source link

w42d3n - Uncontrolled actions after transferral of contract ownership #128

Closed sherlock-admin2 closed 2 months ago

sherlock-admin2 commented 2 months ago

w42d3n

medium

Uncontrolled actions after transferral of contract ownership

Summary

In StrategyPassiveManagerVelodrome.sol, in the retireVault function, unirouter and owner are set to address(0).

Vulnerability Detail

After this, the contract ownership is transferred to the zero address. If the owner is transferred to an address that is not in your control or does not have smart contract capabilities, all functions with onlyOwner modifier will be locked forever , as the contract owner address 0x0 is reserved and cannot be used to send transactions.

Impact

All functions with onlyOwner modifier will be locked forever, for example retireVault(), setOutputToNativePath(), setDeviation(), setTwapInterval(), setPositionWidth(), setRewardPool()

Code Snippet

for example,

https://github.com/sherlock-audit/2024-05-beefy-cowcentrated-liquidity-manager/blob/main/cowcentrated-contracts/contracts/strategies/velodrome/StrategyPassiveManagerVelodrome.sol#L793-L800

function retireVault() external onlyOwner {
    if (IBeefyVaultConcLiq(vault).totalSupply() != 10**3) revert NotAuthorized();
    panic(0,0);
    address feeRecipient = beefyFeeRecipient();
    IERC20Metadata(lpToken0).safeTransfer(feeRecipient, IERC20Metadata(lpToken0).balanceOf(address(this)));
    IERC20Metadata(lpToken1).safeTransfer(feeRecipient, IERC20Metadata(lpToken1).balanceOf(address(this)));
    _transferOwnership(address(0));
}

Tool used

Manual Review

Recommendation

You might want to consider a solution where the contract ownership is transferred to a protocol-owned emergency address from which specific mitigation actions can be taken.

sherlock-admin2 commented 2 months ago

2 comment(s) were left on this issue during the judging contest.

z3s commented:

Invalid; It's intended.

DHTNS commented:

Invalid -> no emergencies after retiring a vault