Delegatecall used without checking target address exists

[ainhoa-a]

Please verify that CALL_ACTOR_ADDRESS or CALL_ACTOR_ID exist before applying delegatecall.

When a non-existing target address is passed to delegatecall it will return true even though the call was not performed.

The low-level functions call, delegatecall and staticcall return true as their first return value if the account called is non-existent, as part of the design of the EVM. Account existence must be checked prior to calling if needed.

• Solidity documentation - Expressions and Control Structures

Delegatecalls are used in utils/Actor.sol to perform callByAddress and callByID to invoke actors:

    (bool success, bytes memory data) = address(CALL_ACTOR_ADDRESS).delegatecall(
        abi.encode(uint64(method_num), value, static_call ? READ_ONLY_FLAG : DEFAULT_FLAG, codec, raw_request, actor_address)
    );

• utils/Actor.sol

  (bool success, bytes memory data) = address(CALL_ACTOR_ID).delegatecall(
      abi.encode(uint64(method_num), value, static_call ? READ_ONLY_FLAG : DEFAULT_FLAG, codec, raw_request, target)
  );

• utils/Actor.sol

If CALL_ACTOR_ADDRESS or CALL_ACTOR_ID does not exist it will cause delegatecall to succeed. As a result of this, the API functions that does not expect any data to be returned will still succeed!

:link: zboto Link

[Stebalien]

The FVM guarantees that this address will exist. Checking if it exists before calling is unnecessary and a waste of gas.

[emmanuelm41]

@Stebalien I know! But this was raised by the auditors!

[Stebalien]

Feel free to reject suggestions by the auditors. They're just that, notices and suggestions. In fact, they can even be dangerous (see https://github.com/Zondax/filecoin-solidity/issues/345).

[Stebalien]

Basically, auditors look for parts of your code that may be dangerous and/or may not behave as you intend. However, at the end of the day, you likely know much more about Filecoin, your code-base, and your users/product.