Incorrect state for accounts without code and nonce

[c4-bot-10]

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/backend/starknet.cairo#L203

Vulnerability details

Impact

When committing the state of an account, if the account has no code and no nonce, nothing is committed. This creates a critical issue when a non-existent account receives some native token. Since nothing is committed for this account (because it has no code and no nonce), its extcodehash will be zero when it should actually be keccak256("").

Proof of Concept

Imagine there’s an address X, which is a non-existent account (no code, no nonce, and no balance). If you get the extcodehash of this address, it returns bytes32(0) as expected. Here’s how the extcodehash operation works: it shows that if the account doesn’t exist, zero is returned, but if the account exists, the hash of the code at address X is returned.

    func exec_extcodehash{
        syscall_ptr: felt*,
        pedersen_ptr: HashBuiltin*,
        range_check_ptr,
        bitwise_ptr: BitwiseBuiltin*,
        stack: model.Stack*,
        memory: model.Memory*,
        state: model.State*,
    }(evm: model.EVM*) -> model.EVM* {
        //................

        let account = State.get_account(evm_address);
        let has_code_or_nonce = Account.has_code_or_nonce(account);
        let account_exists = has_code_or_nonce + account.balance.low + account.balance.high;
        // Relevant cases:
        // https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L392
        if (account_exists == FALSE) {
            Stack.push_uint128(0);
            return evm;
        }

        Stack.push_uint256([account.code_hash]);

        return evm;
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/instructions/environmental_information.cairo#L486

If some native token is sent to address X, calling extcodehash(address(X)) in the same transaction would return keccak256("") because the account now exists (since its balance is no longer zero). This works fine in Kakarot.

Here’s the problem:

Imagine address X, a non-existent account, receives some native token in the first transaction. Then, in a second transaction (not the same transaction), the extcodehash opcode is used to get the code hash at address X. It should return keccak256("") since the account now exists (with a non-zero balance), but instead, it returns bytes32(0) on Kakarot, incorrectly.

The root cause of this issue is as follows:

When the first transaction finishes, the new state of the accounts involved (i.e. sender and Address X) is committed.
The transaction flow looks like this:

eth_rpc::eth_send_raw_unsigned_tx ==> eth_rpc::eth_send_transaction ==> starknet::commit ==> starknet::_commit_accounts ==> starknet::commit

    func _commit_account{
        syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, state: model.State*
    }(self: model.Account*, native_token_address) {
        // .................

        let has_code_or_nonce = Account.has_code_or_nonce(self);
        if (has_code_or_nonce == FALSE) {
            // Nothing to commit
            return ();
        }

        // Set nonce
        IAccount.set_nonce(starknet_address, self.nonce);
        // Save storages
        Internals._save_storage(starknet_address, self.storage_start, self.storage);

        // Update bytecode and jumpdests if required (newly created account)
        if (self.created != FALSE) {
            IAccount.write_bytecode(starknet_address, self.code_len, self.code);
            Internals._save_valid_jumpdests(
                starknet_address, self.valid_jumpdests_start, self.valid_jumpdests
            );
            IAccount.set_code_hash(starknet_address, [self.code_hash]);
            return ();
        }

        return ();
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/backend/starknet.cairo#L203

In this function, when the new state of address X is being committed, it checks if the nonce or code for X is non-zero. Since both are zero (because only native tokens were received, so neither the nonce nor the code changed), nothing is committed. This means the code hash for X (which should now be keccak256("")) is not set.

When the second transaction runs and extcodehash(address(X)) is called, since the balance is non-zero, it skips the if block:

        let account = State.get_account(evm_address);
        let has_code_or_nonce = Account.has_code_or_nonce(account);
        let account_exists = has_code_or_nonce + account.balance.low + account.balance.high;
        // Relevant cases:
        // https://github.com/ethereum/go-ethereum/blob/master/core/vm/instructions.go#L392
        if (account_exists == FALSE) {
            Stack.push_uint128(0);
            return evm;
        }

        Stack.push_uint256([account.code_hash]);

        return evm;

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/instructions/environmental_information.cairo#L486

But since the code hash wasn’t saved after the first transaction, it pushes zero onto the stack instead of keccak256("").

This clearly demonstrates that extcodehash is not functioning the same way as it does on Ethereum. This could have a significant impact depending on the context of different projects. For instance, a malicious user could send 1 wei to any non-existent EOA, ensuring that the extcodehash for that EOA remains incorrectly set to zero. This would mislead any project relying on that information.

PoC

In the following PoC, the test test_reset_extcodehash considers a non-existent account (no code, no nonce, no balance) at the address 0x5b300000009Cf68545DCFcB03FCB875056BEDdC4.

First, the function test1() is executed. Initially, it checks the account’s balance and codehash, showing balance = 0 and codehash = 0. Then, within the same test1() function, 2 wei is transferred to the address, and both the balance and codehash are recalculated. This correctly shows balance = 2 and codehash = keccak256("").

Next, the function test2() is called, which repeats the same process for the same address. It first checks the balance and codehash, correctly showing balance = 2, but incorrectly displays codehash = 0. In test2(), another 2 wei is transferred to the address, and the balance and codehash are recalculated. The balance is correctly shown as 4, but the codehash remains incorrectly at 0.

In the second test, TestResetExtcodehashAddress0, only the balance and extcodehash(address(0)) are checked. It shows that even though the balance is nonzero, the codehash is incorrectly displayed as zero.

The following function was added to the address 2024-09-kakarot\kakarot\kakarot_scripts\utils\kakarot.py, so that when deploying the TestResetExtcodehash contract, 10 wei is just transferred to it initially. This helper simplifies running the main test directly.

async def deployWithValue(
    contract_app: str, contract_name: str, *args, **kwargs
) -> Web3Contract:
    logger.info(f"⏳ Deploying {contract_name}")
    caller_eoa = kwargs.pop("caller_eoa", None)
    contract = await get_contract(contract_app, contract_name, caller_eoa=caller_eoa)
    max_fee = kwargs.pop("max_fee", None)
    value = kwargs.pop("value", 10) # Here it is changed
    gas_price = kwargs.pop("gas_price", DEFAULT_GAS_PRICE)
    receipt, response, success, _ = await eth_send_transaction(
        to=0,
        gas=int(TRANSACTION_GAS_LIMIT),
        data=contract.constructor(*args, **kwargs).data_in_transaction,
        caller_eoa=caller_eoa,
        max_fee=max_fee,
        value=value,
        gas_price=gas_price,
    )
    if success == 0:
        raise EvmTransactionError(bytes(response))

    if WEB3.is_connected():
        evm_address = int(receipt.contractAddress or receipt.to, 16)
        starknet_address = (
            await _call_starknet("kakarot", "get_starknet_address", evm_address)
        ).contract_address
    else:
        starknet_address, evm_address = response
    contract.address = Web3.to_checksum_address(f"0x{evm_address:040x}")
    contract.starknet_address = starknet_address
    logger.info(f"✅ {contract_name} deployed at: {contract.address}")

    return contract

To run the end-to-end tests correctly, the make test-end-to-end7 command should be used, as defined in the Makefile.

test-end-to-end7: deploy
	uv run pytest tests/end_to_end/Simple/test_reset_extcodehash.py -m "not slow" -s --seed 42	

import pytest
import pytest_asyncio

from kakarot_scripts.utils.kakarot import deployWithValue


@pytest_asyncio.fixture(scope="package")
async def target():
    return await deployWithValue(
        "Simple",
        "TestResetExtcodehash",
    )



@pytest.mark.asyncio(scope="package")
class Test:

    class TestResetExtcodehash:
        async def test_reset_extcodehash(self, target):

            await target.test1()

            balanceBefore = await target.balanceBefore()
            balanceAfter = await target.balanceAfter()
            codehashBefore = await target.codehashBefore()
            codehashAfter = await target.codehashAfter()

            print("balanceBefore: " + str(balanceBefore))
            print("balanceAfter: " + str(balanceAfter))
            print("codehashBefore: " + str(codehashBefore.hex()))
            print("codehashAfter: " + str(codehashAfter.hex()))    

            await target.test2()

            balanceBefore2 = await target.balanceBefore2()
            balanceAfter2 = await target.balanceAfter2()
            codehashBefore2 = await target.codehashBefore2()
            codehashAfter2 = await target.codehashAfter2()

            print("balanceBefore2: " + str(balanceBefore2))
            print("balanceAfter2: " + str(balanceAfter2))
            print("codehashBefore2: " + str(codehashBefore2.hex()))
            print("codehashAfter2: " + str(codehashAfter2.hex()))  

    class TestResetExtcodehashAddress0:
        async def test_extcodehash_address_0x00(self, target):        

            codehashAddress0, balanceAddress0 = await target.getAddress0Extcodehash()
            print("balanceAddress0: " + str(balanceAddress0))
            print("codehashAddress0: " + str(codehashAddress0.hex())) 

pragma solidity ^0.8.0;

contract TestResetExtcodehash {
    uint256 public balanceBefore;
    uint256 public balanceAfter;
    bytes32 public codehashBefore;
    bytes32 public codehashAfter;

    uint256 public balanceBefore2;
    uint256 public balanceAfter2;
    bytes32 public codehashBefore2;
    bytes32 public codehashAfter2;

    constructor() payable {}

    function test1() public {
        address addr = 0x5b300000009Cf68545DCFcB03FCB875056BEDdC4;
        bytes32 codehash;
        assembly {
            codehash := extcodehash(addr)
        }
        balanceBefore = addr.balance;
        codehashBefore = codehash;

        (bool success, ) = addr.call{value: 2}("");
        require(success, "success is false");

        assembly {
            codehash := extcodehash(addr)
        }
        balanceAfter = addr.balance;
        codehashAfter = codehash;
    }

    function test2() public {
        address addr = 0x5b300000009Cf68545DCFcB03FCB875056BEDdC4;

        bytes32 codehash;
        assembly {
            codehash := extcodehash(addr)
        }
        balanceBefore2 = addr.balance;
        codehashBefore2 = codehash;

        (bool success, ) = addr.call{value: 2}("");
        require(success, "success is false");

        assembly {
            codehash := extcodehash(addr)
        }
        balanceAfter2 = addr.balance;
        codehashAfter2 = codehash;
    }

    function getAddress0Extcodehash()
        public
        view
        returns (bytes32 codehashAddress0, uint256 balanceAddress0)
    {
        address addr = address(0);
        bytes32 codehash;

        assembly {
            codehash := extcodehash(addr)
        }
        codehashAddress0 = codehash;
        balanceAddress0 = addr.balance;
    }
}

Output log is:

INFO:kakarot_scripts.utils.kakarot:⏳ Deploying TestResetExtcodehash
INFO:kakarot_scripts.utils.kakarot:✅ TestResetExtcodehash deployed at: 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0
INFO:kakarot_scripts.utils.kakarot:⏳ Executing test1() at address 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0
INFO:kakarot_scripts.utils.kakarot:✅ 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0.test1()
balanceBefore: 0
balanceAfter: 2
codehashBefore: 0000000000000000000000000000000000000000000000000000000000000000
codehashAfter: c5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470
INFO:kakarot_scripts.utils.kakarot:⏳ Executing test2() at address 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0
INFO:kakarot_scripts.utils.kakarot:✅ 0x9fE46736679d2D9a65F0992F2272dE9f3c7fa6e0.test2()
balanceBefore2: 2
balanceAfter2: 4
codehashBefore2: 0000000000000000000000000000000000000000000000000000000000000000
codehashAfter2: 0000000000000000000000000000000000000000000000000000000000000000
.balanceAddress0: 352205500000355117
codehashAddress0: 0000000000000000000000000000000000000000000000000000000000000000

Tools Used

Recommended Mitigation Steps

The following modification can solve the issue:

    func _commit_account{
        syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, state: model.State*
    }(self: model.Account*, native_token_address) {
        alloc_locals;

        let is_precompile = PrecompilesHelpers.is_precompile(self.address.evm);
        if (is_precompile != FALSE) {
            return ();
        }

        let starknet_account_exists = Account.is_registered(self.address.evm);
        let starknet_address = self.address.starknet;
        // Case new Account
        if (starknet_account_exists == 0) {
            // Deploy account
            Starknet.deploy(self.address.evm);
            tempvar syscall_ptr = syscall_ptr;
            tempvar pedersen_ptr = pedersen_ptr;
            tempvar range_check_ptr = range_check_ptr;
        } else {
            tempvar syscall_ptr = syscall_ptr;
            tempvar pedersen_ptr = pedersen_ptr;
            tempvar range_check_ptr = range_check_ptr;
        }

        // @dev: EIP-6780 - If selfdestruct on an account created, dont commit data
        // and burn any leftover balance.
        let is_created_selfdestructed = self.created * self.selfdestruct;
        if (is_created_selfdestructed != 0) {
            let starknet_address = Account.compute_starknet_address(Constants.BURN_ADDRESS);
            tempvar burn_address = new model.Address(
                starknet=starknet_address, evm=Constants.BURN_ADDRESS
            );
            let transfer = model.Transfer(self.address, burn_address, [self.balance]);
            State.add_transfer(transfer);
            return ();
        }

        let has_code_or_nonce = Account.has_code_or_nonce(self);
        if (has_code_or_nonce == FALSE) {
            let balance = self.balance.low + self.balance.high;
+           if (balance != FALSE) {
+               IAccount.set_code_hash(starknet_address, [self.code_hash]);
+               return ();
+           }
            // Nothing to commit
            return ();
        }

        // Set nonce
        IAccount.set_nonce(starknet_address, self.nonce);
        // Save storages
        Internals._save_storage(starknet_address, self.storage_start, self.storage);

        // Update bytecode and jumpdests if required (newly created account)
        if (self.created != FALSE) {
            IAccount.write_bytecode(starknet_address, self.code_len, self.code);
            Internals._save_valid_jumpdests(
                starknet_address, self.valid_jumpdests_start, self.valid_jumpdests
            );
            IAccount.set_code_hash(starknet_address, [self.code_hash]);
            return ();
        }

        return ();
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/backend/starknet.cairo#L164

Assessed type

Context

[ClementWalter]

Severity: Medium

Comment: The functions does follow the specs but there is no clear attack path. It will be fixed.

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt changed the severity to QA (Quality Assurance)

[dmvt]

Marking this as QA, functioning according to spec issue. I would have invalidated it entirely for overinflated risk, but it did provide value to the sponsor. As mentioned by the sponsor, no attack vector was provided.

[c4-judge]

dmvt marked the issue as grade-b