010.md

carrot

high

Incorrect threshold update in reconcileSignerCount

Summary

The threshold is incorrectly being updated to validSignerCount. It should instead be updated to newThreshold.

Vulnerability Detail

The function reconcileSignerCount counts the number of valid signers, and updates the threshold of the vault to the expected value. The expected threshold is calculated in the variable newThreshold as seen here https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L198-L202

        if (validSignerCount <= target && validSignerCount != currentThreshold) {
            newThreshold = validSignerCount;
        } else if (validSignerCount > target && currentThreshold < target) {
            newThreshold = target;
        }

However there is a small coding error and when updating the actual threshold, instead of using this value validSignerCount is used, which is different

            bytes memory data = abi.encodeWithSignature("changeThreshold(uint256)", validSignerCount);

This can actually brick the contract completely. This is because in the post-flight check in function checkAfterExecution, the thresholds are compared to the expected value https://github.com/Hats-Protocol/hats-zodiac/blob/9455cc0957762f5dbbd8e62063d970199109b977/src/HatsSignerGateBase.sol#L517-L519

        if (safe.getThreshold() != _getCorrectThreshold()) {
            revert SignersCannotChangeThreshold();
        }

Since the threshold is set incorrectly, this check will revert and break the functionality of the contract.

Impact

Broken contract if more signers present than target threshold

Code Snippet

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

Tool used

Manual Review

Recommendation

Replace validSignerCount with newThreshold