[Security] Integer Underflow Risk in AddressQueueStorage.getIndexOf()

[varunsonavni]

During a security review of the AddressQueueStorage contract, a potential integer underflow vulnerability was identified in the getIndexOf() function. While protected by access controls, this issue could lead to unexpected behavior in queue management operations.

Location: contracts/interface/util/AddressSetStorageInterface.sol getIndexOf() function

Current Implementation:

function getIndexOf(bytes32 _key, address _value) override external view returns (int) {
    int index = int(getUint(keccak256(abi.encodePacked(_key, ".index", _value)))) - 1;
    if (index != -1) {
        index -= int(getUint(keccak256(abi.encodePacked(_key, ".start"))));
        if (index < 0) { index += int(capacity); }
    }
    return index;
}

The function performs an unchecked conversion and subtraction that could potentially underflow in Solidity 0.7.6, which lacks built-in overflow checking for signed integers.
Impact

• Potential return of incorrect index values
• Possible inconsistencies in queue position tracking
• May affect dependent contract operations relying on accurate index values

Risk Assessment

Severity: Low
Complexity: Medium
Exploitability: Low (requires authorized contract access)

Proposed Fix:

function getIndexOf(bytes32 _key, address _value) override external view returns (int) {
    uint256 rawIndex = getUint(keccak256(abi.encodePacked(_key, ".index", _value)));
    if (rawIndex == 0) return -1; // Explicit handling for non-existent items
    
    int index = int(rawIndex - 1);
    if (index != -1) {
        uint256 start = getUint(keccak256(abi.encodePacked(_key, ".start")));
        index -= int(start);
        if (index < 0) { index += int(capacity); }
    }
    return index;
}

While this issue doesn't pose an immediate security risk due to existing access controls, addressing it would improve the contract's robustness and prevent potential integration issues.