Enforce threshold bounds

solidity/contracts/isms/aggregation/StorageAggregationIsm.sol

@@ -43,7 +43,10 @@ contract StorageAggregationIsm is
         address[] memory _modules,
         uint8 _threshold
     ) public onlyOwner {
-        require(_threshold <= _modules.length, "Invalid threshold");
+        require(
+            0 < _threshold && _threshold <= _modules.length,
+            "Invalid threshold"
+        );
         modules = _modules;
         threshold = _threshold;
         emit ModulesAndThresholdSet(_modules, _threshold);
@@ -64,7 +67,7 @@ contract StorageAggregationIsmFactory is
 
     constructor() {
         implementation = address(
-            new StorageAggregationIsm(new address[](0), 0)
+            new StorageAggregationIsm(new address[](1), 1)
         );
     }
 

solidity/contracts/isms/multisig/StorageMultisigIsm.sol

@@ -45,7 +45,10 @@ abstract contract AbstractStorageMultisigIsm is
         address[] memory _validators,
         uint8 _threshold
     ) public onlyOwner {
-        require(_threshold <= _validators.length, "Invalid threshold");
+        require(
+            0 < _threshold && _threshold <= _validators.length,
+            "Invalid threshold"
+        );
         validators = _validators;
         threshold = _threshold;
         emit ValidatorsAndThresholdSet(_validators, _threshold);

solidity/contracts/libs/StaticAddressSetFactory.sol

@@ -36,6 +36,10 @@ abstract contract StaticThresholdAddressSetFactory is
         address[] calldata _values,
         uint8 _threshold
     ) public returns (address) {
+        require(
+            0 < _threshold && _threshold <= _values.length,
+            "Invalid threshold"
+        );
         (bytes32 _salt, bytes memory _bytecode) = _saltAndBytecode(
             _values,
             _threshold

solidity/test/hooks/AggregationHook.t.sol

@@ -22,6 +22,8 @@ contract AggregationHookTest is Test {
         uint8 n,
         uint256 fee
     ) internal returns (address[] memory) {
+        vm.assume(n > 0);
+
         address[] memory hooks = new address[](n);
         for (uint8 i = 0; i < n; i++) {
             TestPostDispatchHook subHook = new TestPostDispatchHook();

solidity/test/isms/AggregationIsm.t.sol

@@ -48,6 +48,8 @@ contract AggregationIsmTest is Test {
         uint8 n,
         bytes32 seed
     ) internal returns (address[] memory) {
+        vm.assume(m > 0 && m <= n && n < 10);
+
         bytes32 randomness = seed;
         address[] memory isms = new address[](n);
         for (uint256 i = 0; i < n; i++) {
@@ -93,7 +95,6 @@ contract AggregationIsmTest is Test {
     }
 
     function testVerify(uint8 m, uint8 n, bytes32 seed) public {
-        vm.assume(0 < m && m <= n && n < 10);
         deployIsms(m, n, seed);
 
         bytes memory metadata = getMetadata(m, seed);
@@ -106,7 +107,7 @@ contract AggregationIsmTest is Test {
         uint8 i,
         bytes32 seed
     ) public {
-        vm.assume(0 < m && m <= n && n < 10 && i < n);
+        vm.assume(i < n);
         deployIsms(m, n, seed);
         (address[] memory modules, ) = ism.modulesAndThreshold("");
         bytes memory noMetadata;
@@ -117,7 +118,6 @@ contract AggregationIsmTest is Test {
     }
 
     function testVerifyMissingMetadata(uint8 m, uint8 n, bytes32 seed) public {
-        vm.assume(0 < m && m <= n && n < 10);
         deployIsms(m, n, seed);
 
         // Populate metadata for one fewer ISMs than needed.
@@ -131,7 +131,6 @@ contract AggregationIsmTest is Test {
         uint8 n,
         bytes32 seed
     ) public {
-        vm.assume(0 < m && m <= n && n < 10);
         deployIsms(m, n, seed);
 
         bytes memory metadata = getMetadata(m, seed);
@@ -143,13 +142,22 @@ contract AggregationIsmTest is Test {
     }
 
     function testModulesAndThreshold(uint8 m, uint8 n, bytes32 seed) public {
-        vm.assume(0 < m && m <= n && n < 10);
         address[] memory expectedIsms = deployIsms(m, n, seed);
         (address[] memory actualIsms, uint8 actualThreshold) = ism
             .modulesAndThreshold("");
         assertEq(abi.encode(actualIsms), abi.encode(expectedIsms));
         assertEq(actualThreshold, m);
     }
+
+    function testZeroThreshold() public {
+        vm.expectRevert("Invalid threshold");
+        factory.deploy(new address[](1), 0);
+    }
+
+    function testThresholdExceedsLength() public {
+        vm.expectRevert("Invalid threshold");
+        factory.deploy(new address[](1), 2);
+    }
 }
 
 contract StorageAggregationIsmTest is AggregationIsmTest {

solidity/test/isms/MultisigIsm.t.sol

@@ -215,6 +215,16 @@ abstract contract AbstractMultisigIsmTest is Test {
         vm.expectRevert("!threshold");
         ism.verify(duplicateMetadata, message);
     }
+
+    function testZeroThreshold() public virtual {
+        vm.expectRevert("Invalid threshold");
+        factory.deploy(new address[](1), 0);
+    }
+
+    function testThresholdExceedsLength() public virtual {
+        vm.expectRevert("Invalid threshold");
+        factory.deploy(new address[](1), 2);
+    }
 }
 
 contract MerkleRootMultisigIsmTest is AbstractMultisigIsmTest {

solidity/test/isms/WeightedMultisigIsm.t.sol

@@ -243,6 +243,14 @@ contract StaticMerkleRootWeightedMultisigIsmTest is
                 seed
             );
     }
+
+    function testThresholdExceedsLength() public override {
+        // no-op
+    }
+
+    function testZeroThreshold() public override {
+        // no-op
+    }
 }
 
 contract StaticMessageIdWeightedMultisigIsmTest is
@@ -298,4 +306,12 @@ contract StaticMessageIdWeightedMultisigIsmTest is
                 seed
             );
     }
+
+    function testThresholdExceedsLength() public override {
+        // no-op
+    }
+
+    function testZeroThreshold() public override {
+        // no-op
+    }
 }