Insuffecient Threshold Validation

[c4-bot-8]

Lines of code

https://github.com/re-nft/smart-contracts/blob/3ddd32455a849c3c6dc3c3aad7a33a6c9b44c291/src/policies/Factory.sol#L138

Vulnerability details

function deployRentalSafe(
    address[] calldata owners,
    uint256 threshold
) external returns (address safe) {
    // Require that the threshold is valid.
    if (threshold == 0 || threshold > owners.length) {
        revert Errors.FactoryPolicy_InvalidSafeThreshold(threshold, owners.length);
    }
    ...
}

The deployRentalSafe function checks that the threshold is greater than 0 and less than or equal to the number of owners. However, if the owners array contains duplicate addresses, the actual number of unique owners could be less than the length of the owners array, which would make the threshold validation inaccurate.

Impact

If the threshold is set based on duplicate entries in the owners array, the safe may not function as intended. For example, if there are 3 unique owners but 5 entries in the owners array with a threshold of 4, the safe would be unusable because it would require more signatures than there are unique owners.

Mitigation

The contract should check for unique addresses in the owners array before setting the threshold. This could be done by using a temporary mapping to track which addresses have already been included:

function deployRentalSafe(
    address[] calldata owners,
    uint256 threshold
) external returns (address safe) {
    require(threshold > 0, "Threshold must be greater than 0");
    // Check for unique owners.
    uint256 uniqueOwnersCount = 0;
    mapping(address => bool) seen;
    for (uint256 i = 0; i < owners.length; i++) {
        if (!seen[owners[i]]) {
            seen[owners[i]] = true;
            uniqueOwnersCount++;
        }
    }
    require(threshold <= uniqueOwnersCount, "Threshold exceeds number of unique owners");

    ...
}

This ensures that the threshold is always within a valid range with respect to the actual number of unique owners.

Assessed type

Invalid Validation

[c4-pre-sort]

141345 marked the issue as insufficient quality report

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality