MohammedRizwan - Claimable gauge distributions are locked when `killGaugeTotally()` is called in `Voter.sol`

[sherlock-admin3]

MohammedRizwan

High

Claimable gauge distributions are locked when killGaugeTotally() is called in Voter.sol

Summary

Claimable gauge distributions are locked when killGaugeTotally() is called in Voter.sol

Vulnerability Detail

In Voter.sol, killGaugeTotally() is used to kill the malicious gauge which is called by emergencyCouncil.

    function killGaugeTotally(address _gauge) external {
        if (msg.sender != emergencyCouncil) {
            require(
                IGaugePlugin(gaugePlugin).checkGaugeKillAllowance(msg.sender, _gauge)
            , "Restart gauge not allowed");
        }
        require(isAlive[_gauge], "gauge already dead");

        address _pool = poolForGauge[_gauge];

        delete isAlive[_gauge];
        delete external_bribes[_gauge];
        delete poolForGauge[_gauge];
        delete isGauge[_gauge];
@>        delete claimable[_gauge];
        delete supplyIndex[_gauge];
        delete gauges[_pool];
        try IPair(_pool).setHasGauge(false) {} catch {}

        killedGauges.push(_gauge);

        emit GaugeKilledTotally(_gauge);
    }

When a gauge is killed, the claimable[_gauge] key value is cleared/deleted i.e it is reset. Because any rewards received by the Voter contract are indexed and distributed in proportion to each pool's weight, this claimable amount is permanently locked within the contract.

This should result in loss of fund, therefore consider returning the claimable amount to the minter address.

27     address public minter;

killGaugeTotally() function and voter contract is heavily inspired from Velodrome's voter contract, This issue is referred for this issue.

Ref: https://github.com/spearbit/portfolio/raw/master/pdfs/Velodrome-Spearbit-Security-Review.pdf

Impact

The distributions to guage claimable will be locked

Code Snippet

https://github.com/sherlock-audit/2024-06-velocimeter/blob/main/v4-contracts/contracts/Voter.sol#L421

Tool used

Manual Review

Recommendation

Consider below changes:

    function killGaugeTotally(address _gauge) external {
        if (msg.sender != emergencyCouncil) {
            require(
                IGaugePlugin(gaugePlugin).checkGaugeKillAllowance(msg.sender, _gauge)
            , "Restart gauge not allowed");
        }
        require(isAlive[_gauge], "gauge already dead");

+        // Return claimable back to minter
+        uint256 _claimable = claimable[_gauge];
+        if (_claimable > 0) {
+            IERC20(base).transfer(minter, _claimable);
+        }

        address _pool = poolForGauge[_gauge];

        delete isAlive[_gauge];
        delete external_bribes[_gauge];
        delete poolForGauge[_gauge];
        delete isGauge[_gauge];
        delete claimable[_gauge];
        delete supplyIndex[_gauge];
        delete gauges[_pool];
        try IPair(_pool).setHasGauge(false) {} catch {}

        killedGauges.push(_gauge);

        emit GaugeKilledTotally(_gauge);
    }

    . . . some code . . .

    function _updateFor(address _gauge) internal {
        address _pool = poolForGauge[_gauge];
        uint256 _supplied = weights[_pool];
        if (_supplied > 0) {
            uint _supplyIndex = supplyIndex[_gauge];
            uint _index = index; // get global index0 for accumulated distro
            supplyIndex[_gauge] = _index; // update _gauge current position to global position
            uint _delta = _index - _supplyIndex; // see if there is any difference that need to be accrued
            if (_delta > 0) {
                uint _share = uint(_supplied) * _delta / 1e18; // add accrued difference for each supplied token
                if (isAlive[_gauge]) {
                    claimable[_gauge] += _share;
-                }
+               } else {
+                    IERC20(base).transfer(minter, _share);        // send rewards back to minter address so they are not stuck in contract
+                }
            }
        } else {
            supplyIndex[_gauge] = index; // new users are set to the default global state
        }
    }

Duplicate of #107