code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L197 https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L190

Vulnerability details

Impact

wrong value returned by numValidators() .

Proof of Concept

The function numValidators() is meant to return the number o validators. if clearValidatorArray() is called the validators array will be cleared but the length will still be the same. https://github.com/code-423n4/2022-09-frax/blob/55ea6b1ef3857a277e2f47d42029bc0f3d6f9173/src/OperatorRegistry.sol#L190

The function setWithdrawalCredential() requires that numValidators() == 0 and the revert message error is "Clear validator array first", but clearing the array will not change the length.

Tools Used

Recommended Mitigation Steps

pop all the validators in the array

FortisFortuna commented 2 years ago

Duplicate of https://github.com/code-423n4/2022-09-frax-findings/issues/260

FortisFortuna commented 2 years ago

Consider the following forge test:

uint256[] internal test_array;

function setUp() public {
       ... Do other stuff;

        test_array.push(1);
        test_array.push(2);
        test_array.push(3);
}

function test_arrayTest() public {
    uint256 arr_length_before = test_array.length;

    delete test_array;

    uint256 arr_length_after = test_array.length;

    require(arr_length_after == 0, 'Array length should be zero');
}

When I ran it, it passed.

0xean commented 2 years ago

closing as invalid.

code-423n4 / 2022-09-frax-findings