function extendedAccountVersion() return wrong supported version for kernel space addresses which has no deployed code

[code423n4]

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/ContractDeployer.sol#L38-L50

Vulnerability details

Impact

Function extendedAccountVersion() Returns the account abstraction version if _address is a deployed contract. when the RawCodeHash of an address is 0x0 then code assumes it is and EOA address and returns AccountAbstractionVersion.Version1 but this would be wrong for kernel space addresses that has no deployed code. the return value of the extendedAccountVersion() would be wrong for those addresses and any logic that uses this function would be vulnerable.

Proof of Concept

This is extendedAccountVersion() code:

    function extendedAccountVersion(address _address) public view returns (AccountAbstractionVersion) {
        AccountInfo memory info = _accountInfo[_address];
        if (info.supportedAAVersion != AccountAbstractionVersion.None) {
            return info.supportedAAVersion;
        }

        // It is an EOA, it is still an account.
        if (ACCOUNT_CODE_STORAGE_SYSTEM_CONTRACT.getRawCodeHash(_address) == 0) {
            return AccountAbstractionVersion.Version1;
        }

        return AccountAbstractionVersion.None;
    }

As you can see when ACCOUNT_CODE_STORAGE_SYSTEM_CONTRACT.getRawCodeHash(_address) == 0 code assumes that the address is and EOA and return AccountAbstractionVersion.Version1, but for kernel space addresses that has no deployed code the raw code hash would be 0 too and the return value of the function would be AccountAbstractionVersion.Version1 which is wrong, because the default code for kernel space addresses that has no deployed code is EmptyContract which doesn't support AccountAbstraction. the wrong return value for those addresses can cause other contract and protocols that rely on this function to detect Account support version to be vulnerable. for example attacker can specify kernel space address to a contract that require AAVersion1 support, and contract would verify the specified address by calling extendedAccountVersion() and call validateTransaction() and executeTransaction() for that address, and because the EmptyContract won't revert so the contract won't detect that the calls are not to valid AAVersion1 supporting address which may result in fund loss. (for example a contract that wants to airdrop tokens to EOA addresses that signed a message)

Tools Used

VIM

Recommended Mitigation Steps

code should check that if specified address is in kernel space then return AccountAbstractionVersion.None before check for raw code hash value.

[miladpiri]

The warden is right about the returned account version for the kernel addresses. However, I disagree with the example. The account version doesn’t mean that account implements it’s methods correctly, so no real impact.

Severity is Low.

[c4-sponsor]

miladpiri marked the issue as disagree with severity

[GalloDaSballo]

The warden has shown how the function doesn't behave as the comment suggests, extendedAccountVersion will return Version1 for addresses in the kernel space.

That said, the function is unused The struct logic implies that any non-contract is an AA, even if no AA contract is deployed

For these reasons, I believe the finding to be a gotcha, meaning QA - Low Severity

[GalloDaSballo]

L

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)