Actors need to maintain two Kakarot accounts for L1 to L2 messaging

[c4-bot-9]

Lines of code

https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/solidity_contracts/src/L1L2Messaging/L1KakarotMessaging.sol#L32-L45
https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/kakarot.cairo#L369-L385
https://github.com/kkrt-labs/kakarot/blob/7411a5520e8a00be6f5243a50c160e66ad285563/src/kakarot/library.cairo#L421-L438

Vulnerability details

Proof of Concept

L1KakarotMessaging

    function sendMessageToL2(address to, uint248 value, bytes calldata data) external payable {
        uint256 totalLength = data.length + 4;
        uint256[] memory convertedData = new uint256[](totalLength);
        convertedData[0] = uint256(uint160(AddressAliasHelper.applyL1ToL2Alias(msg.sender))); <@udit
        convertedData[1] = uint256(uint160(to));
        convertedData[2] = uint256(value);
        convertedData[3] = data.length;
        for (uint256 i = 4; i < totalLength; ++i) {
            convertedData[i] = uint256(uint8(data[i - 4]));
        }

        // Send the converted data to L2
        starknetMessaging.sendMessageToL2{value: msg.value}(kakarotAddress, HANDLE_L1_MESSAGE_SELECTOR, convertedData);
    }

Kakarot

@l1_handler
func handle_l1_message{
    syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr, bitwise_ptr: BitwiseBuiltin*
}(from_address: felt, l1_sender: felt, to_address: felt, value: felt, data_len: felt, data: felt*) {
    alloc_locals;
    Pausable.assert_not_paused();
    let l1_messaging_contract_address = Kakarot.get_l1_messaging_contract_address();
    if (from_address != l1_messaging_contract_address) {
        return ();
    }

    let (_, state, _, _) = Kakarot.handle_l1_message(l1_sender, to_address, value, data_len, data); <@udit

    // Reverted or not - commit the state change. If reverted, the state was cleared to only contain gas-related changes.
    Starknet.commit(state);
    return ();
}

Library

    func handle_l1_message{
        syscall_ptr: felt*,
        pedersen_ptr: HashBuiltin*,
        range_check_ptr,
        bitwise_ptr: BitwiseBuiltin*,
    }(l1_sender: felt, to_address: felt, value: felt, data_len: felt, data: felt*) -> (
        model.EVM*, model.State*, felt, felt
    ) {
        // TODO: ensure fair gas limits and prices
        let (val_high, val_low) = split_felt(value);
        tempvar value_u256 = new Uint256(low=val_low, high=val_high);
        let to = model.Option(is_some=1, value=to_address);
        let (access_list) = alloc();

        return eth_call(
            0, l1_sender, to, 2100000000, 1, value_u256, data_len, data, 0, access_list <@udit
        );
    }

Address aliasing is a common practice in L1 to L2 communication and is used to prevent L1 contracts from impersonating L2 contracts that have the same address.

In this case however, both EOAs and Smart Contracts addresses are aliased, and since there is no bridging involved, this leads to the messages failing unless the Kakarot account corresponding to the aliased address holds enough Starknet's ETH to pay for the Kakarot eth_call gas fees and specified value.

This means that besides their normal Kakarot accounts, actors need to maintain enough funds on their aliased Kakarot accounts too, which they can only directly accessible through L1 to L2 messaging (.e.g you can transfer funds to the account but to get funds out from the account, you need an L1 to L2 message).

Recommended Mitigation Steps

Removing aliasing for EOAs would only fix the issue for EOAs that already bridged to Starknet and hold Starknet's ETH token. This means that a bridging logic is needed to handle both the EOAs that didn't bridge yet and the contracts cases.

Assessed type

Other

[ClementWalter]

Severity: Low

Comment: There is no security risk of any kind. But this will be fixed.

[c4-judge]

dmvt marked the issue as unsatisfactory:
Overinflated severity

[c4-judge]

dmvt marked issue #111 as primary and marked this issue as a duplicate of 111

[c4-judge]

dmvt marked the issue as not a duplicate

[c4-judge]

dmvt marked the issue as unsatisfactory:
Insufficient quality

[imsrybr0]

Hi @dmvt,

Can you please clarify why this was invalidated ?

I see that this was deemed as Overinflated Severity at some point, so I'll answer with regards to that.

The main reason why I believe this is a High is that there is a loss of funds involved.

Starknet L1 to L2 messaging involves sending ETH to incentivize sequencers to relay the message and the ETH sent is consumed in full.

In this case, actors will effectively pay ETH to relay a message, which, while succeeding on the Starknet level, is guaranteed to fail on the Kakarot EVM execution level unless they know beforehand that they need to maintain Starknet's ETH balances on their aliased addresses.

Please review this again, thank you.

[dmvt]

Burning gas / bribes unnecessarily is not a high risk loss of funds. The only way significant funds would be lost is if the user decided to fund the alias, which in my opinion falls under the "conditional on user mistake" ruling.

Regarding insufficient quality, if this were a high risk, I would expect a proof of concept that walks through the exploit and a more robust fix recommendation, not a feature request. Ruling stands.

[imsrybr0]

Hi @dmvt,

Burning gas / bribes unnecessarily is not a high risk loss of funds. The only way significant funds would be lost is if the user decided to fund the alias, which in my opinion falls under the "conditional on user mistake" ruling.

• Paying the sequencer relaying fees is not optional.
• If the user funds the alias no funds are lost as :
  • They would have paid for message that isn't failing by default.
  • They can always use L1 to L2 messages to transfer the funds out from the alias.

Regarding insufficient quality, if this were a high risk, I would expect a proof of concept that walks through the exploit and a more robust fix recommendation, not a feature request. Ruling stands.

You are right, I should have probably added a code proof of concept, I incorrectly assumed that it wasn't required in this case.

As for the recommendations steps, I suggested the removal of EOA aliasing first but also outlined that is only a partial solution to this issue without bridging.

Thanks for taking a look at it again 👍