Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unauthorized calls to Cairo precompiles cause system panic, providing attack opportunities #8

Closed
c4-bot-10 opened this issue Oct 19, 2024 · 4 comments
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b primary issue Highest quality submission among a set of duplicates QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_14_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality

Comments

@c4-bot-10
Copy link
Contributor

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/interpreter.cairo#L1008

Vulnerability details

Impact

When calling Cairo precompiles from an unauthorized address, it causes the system to panic. On Ethereum, this isn't possible if a call encounters an issue, it just uses up all the gas and returns false instead of causing a full system crash. The difference in behavior between Kakarot and Ethereum opens up various attack opportunities, especially on projects like bridges, cross-chain systems, relayers, and more.

Proof of Concept

The Cairo precompiles exist at specific addresses 0x75001 and 0x75002.
https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/precompiles/precompiles_helpers.cairo#L7-L8

When calling these addresses, the function exec_precompile would be called where it jumps to the destination kakarot_precompile. If the caller is not whitelisted/authorized, it jumps to the destination unauthorized_call where it invokes the function unauthorized_precompile.

func exec_precompile{
        syscall_ptr: felt*,
        pedersen_ptr: HashBuiltin*,
        range_check_ptr,
        bitwise_ptr: BitwiseBuiltin*,
    }(
        precompile_address: felt,
        input_len: felt,
        input: felt*,
        caller_code_address: felt,
        caller_address: felt,
    ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) {
        //.........

        let is_kakarot_precompile_ = PrecompilesHelpers.is_kakarot_precompile(precompile_address);
        tempvar syscall_ptr = syscall_ptr;
        tempvar pedersen_ptr = pedersen_ptr;
        tempvar range_check_ptr = range_check_ptr;
        jmp kakarot_precompile if is_kakarot_precompile_ != 0;
        jmp unauthorized_call;

        //...........

        unauthorized_call:
        // Prepare arguments if none of the above conditions are met
        [ap] = syscall_ptr, ap++;
        [ap] = pedersen_ptr, ap++;
        [ap] = range_check_ptr, ap++;
        [ap] = bitwise_ptr, ap++;
        call unauthorized_precompile;
        ret;

        //...........

        kakarot_precompile:
        let is_whitelisted = KakarotPrecompiles.is_caller_whitelisted(caller_code_address);
        tempvar is_not_authorized = 1 - is_whitelisted;
        tempvar syscall_ptr = syscall_ptr;
        tempvar pedersen_ptr = pedersen_ptr;
        tempvar range_check_ptr = range_check_ptr;
        jmp unauthorized_call if is_not_authorized != 0;

        //...........
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/precompiles/precompiles.cairo#L41

In the function unauthorized_precompile, it reverts with error EXCEPTIONAL_HALT which has value 2.

    func unauthorized_precompile{
        syscall_ptr: felt*,
        pedersen_ptr: HashBuiltin*,
        range_check_ptr,
        bitwise_ptr: BitwiseBuiltin*,
    }() -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) {
        let (revert_reason_len, revert_reason) = Errors.unauthorizedPrecompile();
        return (revert_reason_len, revert_reason, 0, Errors.EXCEPTIONAL_HALT);
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/precompiles/precompiles.cairo#L169

    const REVERT = 1;
    const EXCEPTIONAL_HALT = 2;

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/errors.cairo#L10

Returning to the function exec_opcode, it stops the EVM.

    func exec_opcode{
        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 (output_len, output, gas_used, revert_code) = Precompiles.exec_precompile(
                    evm.message.code_address.evm,
                    evm.message.calldata_len,
                    evm.message.calldata,
                    caller_code_address,
                    caller_address,
                );

                let precompile_reverted = is_not_zero(revert_code);
                if (precompile_reverted != FALSE) {
                    // No need to charge gas as precompiles can only trigger EXCEPTIONAL_REVERT
                    // which will consume the entire gas of the context.
                    let evm = EVM.stop(evm, output_len, output, revert_code);
                    tempvar range_check_ptr = range_check_ptr;
                    tempvar evm = evm;
                } else {
                    // Charge gas before stopping
                    let evm = EVM.charge_gas(evm, gas_used);
                    let evm = EVM.stop(evm, output_len, output, evm.reverted);
                    tempvar range_check_ptr = range_check_ptr;
                    tempvar evm = evm;
                }
        //...........
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/interpreter.cairo#L91

Returning to the function run, due to the EXCEPTIONAL_HALT, all the gas would be consumed.

    func run{
        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* {
        alloc_locals;

        if (evm.stopped == FALSE) {
            let evm = exec_opcode(evm);
            return run(evm);
        }

        //........

        if (evm.message.depth == 0) {
            if (evm.reverted != 0) {
                // All REVERTS in a root ctx set the gas_refund to 0.
                // Only if the execution has halted exceptionnaly, consume all gas
                let is_not_exceptional_revert = Helpers.is_zero(evm.reverted - 1);
                let gas_left = is_not_exceptional_revert * evm.gas_left;
                tempvar evm = new model.EVM(
                    message=evm.message,
                    return_data_len=evm.return_data_len,
                    return_data=evm.return_data,
                    program_counter=evm.program_counter,
                    stopped=evm.stopped,
                    gas_left=gas_left,
                    gas_refund=0,
                    reverted=evm.reverted,
                );
                return evm;
            }
            if (evm.message.is_create != FALSE) {
                let evm = Internals._finalize_create_tx(evm);
                return evm;
            }

            return evm;
        }

        //.........
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/interpreter.cairo#L739

Finally, returning to the function execute, it reverts with error EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles.

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

        // Reset the state if the execution has failed.
        // Only the gas fee paid will be committed.
        State.finalize{state=state}();
        if (evm.reverted != 0) {
            with_attr error_message(
                    "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles") {
                assert evm.message.cairo_precompile_called = FALSE;
            }
            tempvar state = State.init();
        } else {
            tempvar state = state;
        }
        //........
    }

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/interpreter.cairo#L1008

The issue is that calling these two contracts will lead to failure An ASSERT_EQ instruction failed: 0 != 1. Even a low-level call, staticcall, w/wo limited forwarded gas will lead to Panic.

On Ethereum, however, if an issue arises, the system does not stop. Instead, the call consumes all the forwarded gas and returns false, without halting the EVM. This is a critical difference that can lead to attacks on systems by misusing Cairo precompiles.

This issue can severely impact projects using Kakarot, such as:

  • Bridges that process multiple messages could be disrupted. For example, if a bridge sends 10 messages to Kakarot and one of them tries to call a precompile address, the entire process stops. Even if the project is designed to handle delivery failures, it could still be attacked this way.

  • Many cross-chain systems rely on callbacks to notify the source when a message has been delivered. If a precompile address is called on the destination (Kakarot), the system will panic and prevent the callback from being sent, leaving the source system pending.

  • Relayers, which handle meta-transactions, could also be affected, causing disruptions in how they process transactions.

PoC

In the following tests, simply two cairo precompile addresses are called. The output log shows the panic.

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

test-end-to-end6: deploy
	uv run pytest tests/end_to_end/Simple/test_cairo_precompile.py -m "not slow" -s --seed 42	
import pytest
import pytest_asyncio

from kakarot_scripts.utils.kakarot import deploy


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



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

    class TestCairoPrecompileAddress0x75001:
        async def test_precompile_address_0x75001(self, target):
            await target.callCairoPrecompile(0x075001)
            result = await target.result()
            print("result of calling address(075001):" + str(result))
            gasDiff = await target.gasDiff()
            print("gas consumption of calling address(075001):" + str(gasDiff))

    class TestCairoPrecompileAddress0x75002:
        async def test_precompile_address_0x75002(self, target):
            await target.callCairoPrecompile(0x075002)
            result = await target.result()
            print("result of calling address(075002):" + str(result))
            gasDiff = await target.gasDiff()
            print("gas consumption of calling address(075002):" + str(gasDiff))
pragma solidity ^0.8.0;

contract TestCairoPrecompiles {
    bool public result;
    uint public gasDiff;

    constructor() payable {}

    function callCairoPrecompile(uint256 _addr) public {
        address precompileAddress = address(uint160(_addr));
        uint initialGas = gasleft();
        (bool success, ) = precompileAddress.call("");
        gasDiff = initialGas - gasleft();
        result = success;
    }
}

The output log is:

E           kakarot_scripts.utils.kakarot.StarknetTransactionError: Starknet tx reverted: Transaction execution has failed:
E           0: Error in the called contract (contract address: 0x029873c310fbefde666dc32a1554fea6bb45eecc84f680f8a2b0a8fbb8cb89af, class hash: 0x05400e90f7e0ae78bd02c77cd75527280470e2fe19c54970dd79dc37a9d3645c, selector: 0x015d40a3d6ca2ac30f4031e42be28da9b056fef9bb7357ac5e85627ee876e5ad):
E           Error at pc=0:4573:
E           Cairo traceback (most recent call last):
E           Unknown location (pc=0:67)
E           Unknown location (pc=0:1835)
E           Unknown location (pc=0:2478)
E           Unknown location (pc=0:3255)
E           Unknown location (pc=0:3795)
E           
E           1: Error in the called contract (contract address: 0x06aa020edda5facede4e7e34066650883193b6b800a2f38cfa0b247cec925e78, class hash: 0x07b2de5e73ff7eb338d76c967dd5aa3f3004574d326b8c1402bb819d4983b8b6, selector: 0x007ec457cd7ed1630225a8328f826a29a327b19486f6b2882b4176545ebdbe3d):
E           Error at pc=0:22:
E           Cairo traceback (most recent call last):
E           Unknown location (pc=0:273)
E           Unknown location (pc=0:259)
E           
E           2: Error in a library call (contract address: 0x06aa020edda5facede4e7e34066650883193b6b800a2f38cfa0b247cec925e78, class hash: 0x01c8acbb634afbe8e494faba9ca2d8c6ad385372d5bfdd32a88506fffc709707, selector: 0x007ec457cd7ed1630225a8328f826a29a327b19486f6b2882b4176545ebdbe3d):
E           Error at pc=0:32:
E           Cairo traceback (most recent call last):
E           Unknown location (pc=0:3247)
E           Unknown location (pc=0:3180)
E           Unknown location (pc=0:2614)
E           Unknown location (pc=0:445)
E           
E           3: Error in the called contract (contract address: 0x01eecd3ea316cfef4109a1fc452784038970699ff1d9e4b7f60b90ff13c73e8a, class hash: 0x07f77b2f1b6710e7baa7cf71af86172f85e99135c9a0625c15cf013eea86ad19, selector: 0x01df22f179266780b29ce26485abf97cbd909fd15ce74a37b793bcd261f84b14):
E           Error message: EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles
E           Error at pc=0:15917:
E           Cairo traceback (most recent call last):
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23354)
E           Unknown location (pc=0:23460)
E           
E           An ASSERT_EQ instruction failed: 0 != 1.

Tools Used

Recommended Mitigation Steps

To address this problem, the precompile logic should be updated to align more closely with Ethereum’s behavior. Specifically, the system should avoid halting entirely when an unauthorized address attempts to call a precompile.

Assessed type

Context

@c4-bot-10 c4-bot-10 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Oct 19, 2024
c4-bot-4 added a commit that referenced this issue Oct 19, 2024
@c4-bot-13 c4-bot-13 added the 🤖_14_group AI based duplicate group recommendation label Oct 25, 2024
@howlbot-integration howlbot-integration bot added primary issue Highest quality submission among a set of duplicates sufficient quality report This report is of sufficient quality labels Oct 27, 2024
@ClementWalter
Copy link

Severity: Invalid

Comment: Not reverting the tx if the caller is not authorized could be fixed but we do not understand the clear attack path here. This behavior will be added to the documentation.

@ClementWalter ClementWalter added the sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue label Nov 4, 2024
@dmvt
Copy link

dmvt commented Nov 7, 2024

I also don't see the attack vector here. Downgrading to informational given that it is a documentation issue.

@c4-judge c4-judge added downgraded by judge Judge downgraded the risk level of this issue QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Nov 7, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Nov 7, 2024

dmvt changed the severity to QA (Quality Assurance)

@c4-judge
Copy link
Contributor

c4-judge commented Nov 8, 2024

dmvt marked the issue as grade-b

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue grade-b primary issue Highest quality submission among a set of duplicates QA (Quality Assurance) Assets are not at risk. State handling, function incorrect as to spec, issues with clarity, syntax 🤖_14_group AI based duplicate group recommendation sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

6 participants