sherlock-audit / 2023-02-hats-judging

2 stars 0 forks source link

Bauer - The target threshold may be lower than the minimum threshold #15

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Bauer

medium

The target threshold may be lower than the minimum threshold

Summary

Missing check for _targetThreshold > minThreshold in the setTargetThreshold() function which could result in a target threshold that is lower than the minimum threshold

Vulnerability Detail

The setTargetThreshold() function is used to set a new target threshold ,however there is no check _targetThreshold > minThreshold which could result in a target threshold that is lower than the minimum threshold and the _getCorrectThreshold function will get the incorrect value.

    function setTargetThreshold(uint256 _targetThreshold) public onlyOwner {
        if (_targetThreshold != targetThreshold) {
            _setTargetThreshold(_targetThreshold);

            if (signerCount > 1) _setSafeThreshold(_targetThreshold);

            emit HSGLib.TargetThresholdSet(_targetThreshold);
        }
    }

    /// @notice Internal function to set the target threshold
    /// @dev Reverts if `_targetThreshold` is greater than `maxSigners`
    /// @param _targetThreshold The new target threshold to set
    function _setTargetThreshold(uint256 _targetThreshold) internal {
        if (_targetThreshold > maxSigners) {
            revert InvalidTargetThreshold();
        }

        targetThreshold = _targetThreshold;
    }

    function _getCorrectThreshold() internal view returns (uint256 _threshold) {
        uint256 count = _countValidSigners(safe.getOwners());
        uint256 min = minThreshold;
        uint256 max = targetThreshold;
        if (count < min) _threshold = min;
        else if (count > max) _threshold = max;
        else _threshold = count;
    }

Impact

The target threshold may be lower than the minimum threshold

Code Snippet

https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L95-L114

Tool used

Manual Review

Recommendation

   function _setTargetThreshold(uint256 _targetThreshold) internal {
        require(_targetThreshold > minThreshold,"FAILED");
        if (_targetThreshold > maxSigners) {
            revert InvalidTargetThreshold();
        }

        targetThreshold = _targetThreshold;
    }

Duplicate of #36