Swap Signer fails if final owner is invalid due to off by one error in loop
New users attempting to call `claimSigner()` when there is already a full slate of owners are supposed to kick any invalid owners off the safe in order to swap in and take their place. However, the loop that checks this has an off-by-one error that misses checking the final owner.\\nWhen `claimSigner()` is called, it adds the `msg.sender` as a signer, as long as there aren't already too many owners on the safe.\\nHowever, in the case that there are already the maximum number of owners on the safe, it performs a check whether any of them are invalid. If they are, it swaps out the invalid owner for the new owner.\\n```\\nif (ownerCount >= maxSigs) {\\n    bool swapped = _swapSigner(owners, ownerCount, maxSigs, currentSignerCount, msg.sender);\\n    if (!swapped) {\\n        // if there are no invalid owners, we can't add a new signer, so we revert\\n        revert NoInvalidSignersToReplace();\\n    }\\n}\\n```\\n\\n```\\nfunction _swapSigner(\\n    address[] memory _owners,\\n    uint256 _ownerCount,\\n    uint256 _maxSigners,\\n    uint256 _currentSignerCount,\\n    address _signer\\n) internal returns (bool success) {\\n    address ownerToCheck;\\n    bytes memory data;\\n\\n    for (uint256 i; i < _ownerCount - 1;) {\\n        ownerToCheck = _owners[i];\\n\\n        if (!isValidSigner(ownerToCheck)) {\\n            // prep the swap\\n            data = abi.encodeWithSignature(\\n                "swapOwner(address,address,address)",\\n                _findPrevOwner(_owners, ownerToCheck), // prevOwner\\n                ownerToCheck, // oldOwner\\n                _signer // newOwner\\n            );\\n\\n            // execute the swap, reverting if it fails for some reason\\n            success = safe.execTransactionFromModule(\\n                address(safe), // to\\n                0, // value\\n                data, // data\\n                Enum.Operation.Call // operation\\n            );\\n\\n            if (!success) {\\n                revert FailedExecRemoveSigner();\\n            }\\n\\n            if (_currentSignerCount < _maxSigners) ++signerCount;\\n            break;\\n        }\\n        unchecked {\\n            ++i;\\n        }\\n    }\\n}\\n```\\n\\nThis function is intended to iterate through all the owners, check if any is no longer valid, and — if that's the case — swap it for the new one.\\nHowever, in the case that all owners are valid except for the final one, it will miss the swap and reject the new owner.\\nThis is because there is an off by one error in the loop, where it iterates through `for (uint256 i; i < _ownerCount - 1;)...`\\nThis only iterates through all the owners up until the final one, and will miss the check for the validity and possible swap of the final owner.
Perform the loop with `ownerCount` instead of `ownerCount - 1` to check all owners:\\n```\\n// Remove the line below\\n for (uint256 i; i < _ownerCount // Remove the line below\\n 1;) {\\n// Add the line below\\n for (uint256 i; i < _ownerCount ;) {\\n     ownerToCheck = _owners[i];\\n    // rest of code\\n}\\n```\\n
When only the final owner is invalid, new users will not be able to claim their role as signer, even through they should.
```\\nif (ownerCount >= maxSigs) {\\n    bool swapped = _swapSigner(owners, ownerCount, maxSigs, currentSignerCount, msg.sender);\\n    if (!swapped) {\\n        // if there are no invalid owners, we can't add a new signer, so we revert\\n        revert NoInvalidSignersToReplace();\\n    }\\n}\\n```\\n