Fix EOA check heuristics

[RomarQ]

Follow-up of: rust-ethereum/evm#313

Implements a new method on the precompile handler to obtain the addresses of contracts being constructed.

This is necessary because contracts under-construction don't have code, the code will only be added to storage after the init_code is executed.

Address type heuristics:

• It is a smart-contract if the address is a contract being constructed;
• It is a Precompile if calling pallet_evm::Config::PrecompilesValue::get().is_precompile returns true;
• It is a EOA when the address does not have code;
• If neither of the conditions above are true, we assume that an address is a smart-contract.

[sorpaas]

What happens when CREATE fails? Is this heuristic still accurate?

[sorpaas]

For supporting EIP-7702 we will probably have to rely on tx.origin to ensure an address is a EOA, since code_len == 0 |= EOA will no longer be valid.

I'm not certain if there's a specification issue in EIP-7702 (and we should ping them immediately about this because it's in last call!), or we misunderstood this.

In "Behavior" section No. 5:

Verify the code of authority is empty or already delegated.

EIP-7702 only checks that the code is empty. "Already delegated" means that a "delegation indicator" is written in the account code (0xef0100 || address), and in this case the code is also not empty.

I think what they're doing is already sufficient because EIP-7702 is on transaction level. You can't invoke a CREATE at the same time.

[RomarQ]

For supporting EIP-7702 we will probably have to rely on tx.origin to ensure an address is a EOA, since code_len == 0 |= EOA will no longer be valid.

I'm not certain if there's a specification issue in EIP-7702 (and we should ping them immediately about this because it's in last call!), or we misunderstood this.

In "Behavior" section No. 5:

Verify the code of authority is empty or already delegated.

EIP-7702 only checks that the code is empty. "Already delegated" means that a "delegation indicator" is written in the account code (0xef0100 || address), and in this case the code is also not empty.

I think what they're doing is already sufficient because EIP-7702 is on transaction level. You can't invoke a CREATE at the same time.

For being compliant with EIP-7702, checking if an address is EOA could be the following:

• If EXTCODESIZE is 23 and code prefix is 0xef0100;
• Or if code_len == 0.

The purpose of this PR is not to add support for EIP-7702, but instead to fix a issue, where contracts being created were considered EOA, since the code is only set to the state once the init code has been executed.

Updated the PR description to avoid confusion, fn origin() still makes sense to be exposed to precompiles.

[sorpaas]

Can you explain more what happens when CREATE fails? In this case the code is still supposed to be empty.

[RomarQ]

Can you explain more what happens when CREATE fails? In this case the code is still supposed to be empty.

Even if CREATE or CREATE2 fails, this check will give true for the whole duration of the transaction. Which is fine, since we know that it is an address derived from a CREATE, CREATE2 call.

The notion about created contracts was added in (PR #1588) for supporting https://eips.ethereum.org/EIPS/eip-6780.

[sorpaas]

@RomarQ Is this SRLabs' S3-55? From what I see this doesn't directly affect Frontier, as CallableByContract is Moonbeam-specific. Can you confirm?

[RomarQ]

@RomarQ Is this SRLabs' S3-55? From what I see this doesn't directly affect Frontier, as CallableByContract is Moonbeam-specific. Can you confirm?

Yes, it fixes S3-55.
Not sure if other teams are also using CallableByContract, since it is now provided from precompile-utils in frontier

[Cayo-srlabs]

The underlying issue applied to Frontier since it was a core-assumption which was broken. Any security sensitive features relying on inspecting the account type would be rendered insufficient. This extends to any custom or native features dependent on this underlying check, as an example CallableByContract.

[sorpaas]

Is any precompile implementation in Frontier that uses AddressType vulnerable? If not I'll set the severity to low, otherwise I'll set it to medium/moderate.

[RomarQ]

I would say that the severity is low

[sorpaas]

Security advisory: GHSA-fr62-ppwc-mc2h