code-423n4 / 2022-07-axelar-findings

0 stars 0 forks source link

QA Report #168

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Did Not Check If The operators and weights Array Length Is The Same

The AxelarAuthWeighted._validateSignatures function did not validate that the operators and weights array length are the same.

https://github.com/code-423n4/2022-07-axelar/blob/3729dd4aeff8dc2b8b9c3670a1c792c81fc60e7c/contracts/auth/AxelarAuthWeighted.sol#L86

function _validateSignatures(
    bytes32 messageHash,
    address[] memory operators,
    uint256[] memory weights,
    uint256 threshold,
    bytes[] memory signatures
) internal pure {
    uint256 operatorsLength = operators.length;
    uint256 operatorIndex = 0;
    uint256 weight = 0;
    // looking for signers within operators
    // assuming that both operators and signatures are sorted
    for (uint256 i = 0; i < signatures.length; ++i) {
        address signer = ECDSA.recover(messageHash, signatures[i]);
        // looping through remaining operators to find a match
        for (; operatorIndex < operatorsLength && signer != operators[operatorIndex]; ++operatorIndex) {}
        // checking if we are out of operators
        if (operatorIndex == operatorsLength) revert MalformedSigners();
        // return if weight sum above threshold
        weight += weights[operatorIndex];
        // weight needs to reach or surpass threshold
        if (weight >= threshold) return;
        // increasing operators index if match was found
        ++operatorIndex;
    }
    // if weight sum below threshold
    revert MalformedSigners();
}

Recommendation

Implement the following check to ensure that the operators and weights array length are the same.

require(operators.length == weights.length, "operators and weights length not the same")
re1ro commented 2 years ago

Not an issue. It's checked in _transferOperatorship Dup #16

GalloDaSballo commented 2 years ago

Per the sponsor comment, as well as: https://github.com/code-423n4/2022-07-axelar/blob/9c4c44b94cddbd48b9baae30051a4e13cbe39539/contracts/auth/AxelarAuthWeighted.sol#L103-L104

            if (operatorIndex == operatorsLength) revert MalformedSigners();

If the length surpasses operatorsLength we'll get a revert, meaning that while the check will help with an earlier revert, it wont' cause any vulnerability.

I think because early failure is a coding convention, the finding is a valid Refactoring but not a vulnerability