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 Contracts Can Bypass Precompile Authorization via delegatecall in Kakarot zkEVM #124

Open
howlbot-integration bot opened this issue Oct 28, 2024 · 2 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden H-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_34_group AI based duplicate group recommendation selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/kkrt-labs/kakarot/blob/6496ee1d18380dae5b069c27b3129d82ae282419/cairo_zero/kakarot/precompiles/precompiles.cairo#L29-L161

Vulnerability details

Summary

In the Kakarot zkEVM implementation exec_precompile, unauthorized contracts can bypass the precompile authorization mechanism using the delegatecall. This allows unauthorized contracts to execute privileged precompile functions intended only for authorized (whitelisted) contracts, compromising the security and integrity of the system.

Vulnerability Details:

Let's take a look at the exec_precompile function with the unessential part omitted for brevity:

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) {

    //SNIP...
    // Authorization for Kakarot precompiles
    let is_kakarot_precompile = PrecompilesHelpers.is_kakarot_precompile(precompile_address);
    if is_kakarot_precompile != 0 {
        // Check if caller is whitelisted
        let is_whitelisted = KakarotPrecompiles.is_caller_whitelisted(caller_code_address);
        if is_whitelisted == FALSE {
            jmp unauthorized_call;
        }
        // Proceed with precompile execution
        // ...
    } else {
        jmp unauthorized_call;
    }
}

The issue arises because the authorization mechanism relies solely on caller_code_address, which refers to the code being executed, and does not consider caller_address, the actual address of the contract making the call. This approach is insufficient and can be exploited using delegatecall.

Analogy:
If an EVM contract A delegatecalls EVM contract B, and B is allowlisted, then the call to the Cairo precompile succeeds because the allowlist is based on the code address (of B) and not on the execution address (of A).
This means that an unauthorized contract (Contract A) can exploit this behavior to execute privileged operations by delegatecalling a whitelisted contract (Contract B) and passing the authorization check incorrectly.

As such, it is an issue for other whitelisted smart contracts allowing arbitrary calls to external smart contracts. For example, this is the case of some smart contracts allowing callbacks. In those situations, a malicious user could make a DEX execute a call to the malicious smart contract that would be able to access the precompiles pretending to be the DEX.

Impact

This issue could allow unauthorized contracts to perform operations that should be restricted to whitelisted contracts like in the case of DEX that supports callbacks and has been added to the allowistlist. Moreover, all contracts within the allowistlist must be written with the assumption that they can also be delegatecalled; i.e they must not make any assumptions about their storage or their own EVM address (address(this)).

Prior to my discussion with the sponsor in a private thread about the implications of this attack surface, the sponsor intended to eventually remove the whitelist mechanism, not fully considering the dangers of delegatecalls to precompiles, which would have been a bad idea.

And with the team’s plan to eventually remove the allowlist, prior to knowing about this attack surface, it gives any contract the opportunity to exploit this on KakarotEVM, which, as we’ve seen, can be catastrophic—especially in the case of MoonBeam’s msg.sender impersonation disclosure by pwning.eth

Recommendation

In the interim, the obvious mitigation is to ensure that all whitelisted contracts are written with the assumption that they can also be delegatecalled. And more importantly, the whitelist mechanism should never be removed as this would open a flood gate of vulnerabilities to the protocol.

A permanent solution to this issue would be to disable Delegatecalls for custom precompiles.

Assessed type

call/delegatecall

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_34_group AI based duplicate group recommendation 🤖_primary AI based primary recommendation bug Something isn't working duplicate-38 edited-by-warden sufficient quality report This report is of sufficient quality labels Oct 28, 2024
howlbot-integration bot added a commit that referenced this issue Oct 28, 2024
@ClementWalter
Copy link

ClementWalter commented Nov 1, 2024

Severity: High

Comment: Ok dup #38 closed by bot

@ClementWalter ClementWalter added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Nov 4, 2024
@c4-judge c4-judge reopened this Nov 8, 2024
@c4-judge c4-judge added primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report labels Nov 8, 2024
@c4-judge
Copy link
Contributor

c4-judge commented Nov 8, 2024

dmvt marked the issue as selected for report

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working edited-by-warden H-01 primary issue Highest quality submission among a set of duplicates 🤖_primary AI based primary recommendation 🤖_34_group AI based duplicate group recommendation selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

3 participants