fix: Update NonMatchingSelector tests

[AmadiMichael]

Great work here guys! just wanted to contribute to fixes to my findings

/// @notice Test that a non-matching selector reverts
  function testNonMatchingSelector(bytes32 callData) public {
    bytes8[] memory func_selectors = new bytes8[](2);
    func_selectors[0] = bytes8(hex"13af4035");
    func_selectors[1] = bytes8(hex"8da5cb5b");

    bytes8 func_selector = bytes8(callData >> 0xe0);
    for (uint256 i = 0; i < 2; i++) {
      if (func_selector != func_selectors[i]) {
        return;
      }
    }

    address target = address(owner);
    uint256 OneWord = 0x20;
    bool success = false;
    assembly {
      success := staticcall(
          gas(),
          target,
          add(callData, OneWord),
          mload(callData),
          0,
          0
      )
    }
    assert(!success);
  }

This test found in most auth tests is bound to always succeed without really testing it, actually there's no chance a call to this function would reach the assembly part.

Example: if the fuzzer parses in 0xb10e2d527612073b26eecdfd717e6a320cf44b4afac2b0732d9fcbe2b7fa0cf6, the array is filled up with the function sigs the contract has but then, firstly this line bytes8 func_selector = bytes8(callData >> 0xe0); is bound to return 0x0000000000000000. Using the bytes32 above this will firstly execute (0xb10e2d527612073b26eecdfd717e6a320cf44b4afac2b0732d9fcbe2b7fa0cf6 >> 0xe0) which is 0xb10e2d52 with 224 bits of zero to the left. Performing bytes8(0xb10e2d52) will only take the first 8 bytes of this which is all zeroes.

I fixed it in assembly with

bytes4 func_selector;
        assembly {
            func_selector := and(
                callData,
                0xffffffff00000000000000000000000000000000000000000000000000000000
            )
        }

Secondly, even if the above issue didn't exist, the if statement states that if (func_selector != func_selectors[i]) { return; } while looping through the array of signatures. The issue is that rather than checking that if the shifted bytes above returns the same bytes as a function sig in the array it should return as that's not the aim of the test but rather it returns if the shifted bytes is different from any one in the array meaning that it would only get to the assembly part if the sig is the same and which will obviously pass. The fix was to change != to ==

The third issue was the assembly part. It wasn't detected i believe because the test never got there but if it did the execution would have ran out of gas. In the call it asks the evm to read from add(callData, OneWord) which could be as much as uint256.max + 0x80 also it asks it to read up to the latter plus mload(calldata) which could be doubling the size minus 0x80.

The fix was this

bool success = false;
        assembly {
            mstore(0x80, callData)
            // if its a state changing call this will return 0
            success := staticcall(gas(), target, 0x80, 0x20, 0, 0)

            if iszero(success) {
                success := call(gas(), target, 0, 0x80, 0x20, 0, 0)
            }
        }

Checking a static call first incase which if it fails we make a call. mstoring calldata in memory and using it.

The second was the TSOwnable failing test (without a need to add config_with_create_2), by separating the HuffConfig deployment and storing the returned address.

[MathisGD]

Great finding ! A few comments :

[Maddiaa0]

Nice, thank you both @MathisGD and @AmadiMichael