From 8e37d05afeaed635eb07b0f3ab996ad49daa95d8 Mon Sep 17 00:00:00 2001 From: Mathieu <60658558+enitrat@users.noreply.github.com> Date: Fri, 15 Nov 2024 04:30:31 +0100 Subject: [PATCH] [KGA-29] [KGA-136] fix: cases with Cairo precompiles and delegatecalls (#1567) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the issue raised during the audit about eventual honeypots that could lure a user and manipulate starknet tokens without explicit approvals 1. Remove dependence on the `code_address` for whitelisting, which were reported dangerous. 2. Disable ability of having nesting a delegatecall to 0x75001 at protocol level. (no longer possible to do User A -call-> contract C -delegatecall->DualVmToken -delegatecall->0x75001 as a byproduct of 1. 3. Disable ability of calling 0x75002 with a DELEGATECALL at protocol level 4. Added noDelegateCall modifier to L2KakarotMessaging and DualVMToken for extra security 4. Added associated tests under `Security/` https://github.com/code-423n4/2024-09-kakarot-findings/issues/38 Closes #1562 - - - This change is [Reviewable](https://reviewable.io/reviews/kkrt-labs/kakarot/1567) --------- Co-authored-by: Oba Co-authored-by: Clément Walter --- cairo_zero/kakarot/interpreter.cairo | 17 +- .../kakarot/precompiles/precompiles.cairo | 4 +- .../precompiles/precompiles_helpers.cairo | 22 +- .../precompiles/test_precompiles.cairo | 3 - .../kakarot/precompiles/test_precompiles.py | 47 ++- kakarot_scripts/utils/kakarot.py | 3 +- .../CallCairoPrecompileTest.sol | 2 +- .../src/CairoPrecompiles/DualVmToken.sol | 26 +- .../src/L1L2Messaging/L2KakarotMessaging.sol | 8 +- .../src/NoDelegateCall/DualVmTokenHack.sol | 80 ++++ .../DualVmTokenWithoutModifier.sol | 372 ++++++++++++++++++ .../NoDelegateCall/L2KakarotMessagingHack.sol | 18 + .../src/NoDelegateCall/NoDelegateCall.sol | 28 ++ .../test_call_cairo_precompile.py | 10 +- .../test_multicall_cairo_precompile.py | 10 +- .../test_whitelisted_call_cairo_precompile.py | 6 +- .../NoDelegateCall/test_dual_vm_token_hack.py | 174 ++++++++ .../NoDelegateCall/test_l2_messaging_hack.py | 35 ++ tests/end_to_end/conftest.py | 2 +- 19 files changed, 783 insertions(+), 84 deletions(-) create mode 100644 solidity_contracts/src/NoDelegateCall/DualVmTokenHack.sol create mode 100644 solidity_contracts/src/NoDelegateCall/DualVmTokenWithoutModifier.sol create mode 100644 solidity_contracts/src/NoDelegateCall/L2KakarotMessagingHack.sol create mode 100644 solidity_contracts/src/NoDelegateCall/NoDelegateCall.sol create mode 100644 tests/end_to_end/NoDelegateCall/test_dual_vm_token_hack.py create mode 100644 tests/end_to_end/NoDelegateCall/test_l2_messaging_hack.py diff --git a/cairo_zero/kakarot/interpreter.cairo b/cairo_zero/kakarot/interpreter.cairo index 252e28f95..c9e1da377 100644 --- a/cairo_zero/kakarot/interpreter.cairo +++ b/cairo_zero/kakarot/interpreter.cairo @@ -67,20 +67,11 @@ namespace Interpreter { let is_precompile = PrecompilesHelpers.is_precompile(evm.message.code_address.evm); if (is_precompile != FALSE) { let parent_context = evm.message.parent; - let is_parent_zero = Helpers.is_zero(cast(parent_context, felt)); - if (is_parent_zero != FALSE) { - // Case A: The precompile is called straight from an EOA - tempvar caller_code_address = evm.message.caller; - } else { - // Case B: The precompile is called from a contract - tempvar caller_code_address = parent_context.evm.message.code_address.evm; - } tempvar caller_address = evm.message.caller; 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, evm.message.address.evm, ); @@ -101,9 +92,15 @@ namespace Interpreter { } let range_check_ptr = [ap - 2]; let evm = cast([ap - 1], model.EVM*); + + // Only count the cairo precompile if it was executed and did not revert. + // If it did revert, we're ensured no state changes were made in the cairo call. let is_cairo_precompile_called = PrecompilesHelpers.is_kakarot_precompile( evm.message.code_address.evm ); + let is_cairo_precompile_executed = is_cairo_precompile_called * ( + 1 - precompile_reverted + ); tempvar message = new model.Message( bytecode=evm.message.bytecode, bytecode_len=evm.message.bytecode_len, @@ -120,7 +117,7 @@ namespace Interpreter { is_create=evm.message.is_create, depth=evm.message.depth, env=evm.message.env, - cairo_precompile_called=is_cairo_precompile_called, + cairo_precompile_called=is_cairo_precompile_executed, ); tempvar evm = new model.EVM( message=message, diff --git a/cairo_zero/kakarot/precompiles/precompiles.cairo b/cairo_zero/kakarot/precompiles/precompiles.cairo index 2d8816e55..4704f2a88 100644 --- a/cairo_zero/kakarot/precompiles/precompiles.cairo +++ b/cairo_zero/kakarot/precompiles/precompiles.cairo @@ -32,7 +32,6 @@ namespace Precompiles { // @param precompile_address The precompile evm_address. // @param input_len The length of the input array. // @param input The input array. - // @param caller_code_address The address of the code of the contract that calls the precompile. // @param caller_address The address of the caller of the precompile. Delegatecall rules apply. // @param message_address The address being executed in the current message. // @return output_len The output length. @@ -48,7 +47,6 @@ namespace Precompiles { precompile_address: felt, input_len: felt, input: felt*, - caller_code_address: felt, caller_address: felt, message_address: felt, ) -> (output_len: felt, output: felt*, gas_used: felt, reverted: felt) { @@ -135,7 +133,7 @@ namespace Precompiles { kakarot_precompile: let is_call_authorized_ = PrecompilesHelpers.is_call_authorized( - precompile_address, caller_code_address, caller_address, message_address + precompile_address, caller_address, message_address ); tempvar is_not_authorized = 1 - is_call_authorized_; tempvar syscall_ptr = syscall_ptr; diff --git a/cairo_zero/kakarot/precompiles/precompiles_helpers.cairo b/cairo_zero/kakarot/precompiles/precompiles_helpers.cairo index bbb7b352b..168c3cc46 100644 --- a/cairo_zero/kakarot/precompiles/precompiles_helpers.cairo +++ b/cairo_zero/kakarot/precompiles/precompiles_helpers.cairo @@ -75,25 +75,30 @@ namespace PrecompilesHelpers { // @notice Returns whether the call to the precompile is authorized. // @dev A call is authorized if: - // a. The precompile requires a whitelist AND the CODE_ADDRESS of the caller is whitelisted + // a. The precompile requires a whitelist AND the ADDRESS of the caller is whitelisted // b. The precompile is CAIRO_MULTICALL_PRECOMPILE and the precompile address is the same as the message address (NOT a DELEGATECALL / CALLCODE). // @param precompile_address The address of the precompile. - // @param caller_code_address The code_address of the precompile caller. // @param caller_address The address of the caller. // @param message_address The address being executed in the current message. // @return Whether the call is authorized. func is_call_authorized{syscall_ptr: felt*, pedersen_ptr: HashBuiltin*, range_check_ptr}( - precompile_address: felt, - caller_code_address: felt, - caller_address: felt, - message_address: felt, + precompile_address: felt, caller_address: felt, message_address: felt ) -> felt { alloc_locals; let precompile_requires_whitelist = requires_whitelist(precompile_address); + tempvar is_not_delegatecall: felt = Helpers.is_zero(message_address - precompile_address); + // Ensure that calls to precompiles that require a whitelist are properly authorized. if (precompile_requires_whitelist == TRUE) { - let is_whitelisted = is_caller_whitelisted(caller_code_address); + // If the call is not a DELEGATECALL / CALLCODE, the actual caller is the caller_address + // Otherwise, the actual caller is the message_address. + if (is_not_delegatecall != FALSE) { + tempvar actual_caller_address = caller_address; + } else { + tempvar actual_caller_address = message_address; + } + let is_whitelisted = is_caller_whitelisted(actual_caller_address); tempvar syscall_ptr = syscall_ptr; tempvar pedersen_ptr = pedersen_ptr; tempvar range_check_ptr = range_check_ptr; @@ -109,11 +114,10 @@ namespace PrecompilesHelpers { let range_check_ptr = [ap - 2]; let authorized = [ap - 1]; - // Ensure that calls to CAIRO_CALL_PRECOMPILE or CAIRO_CALL_PRECOMPILE are not made through + // Ensure that calls to CAIRO_CALL_PRECOMPILE or CAIRO_MULTICALL_PRECOMPILE are not made through // a delegatecall / callcode. let is_delegatecall_protected_ = is_delegatecall_protected(precompile_address); if (is_delegatecall_protected_ != FALSE) { - let is_not_delegatecall = Helpers.is_zero(message_address - precompile_address); tempvar authorized = authorized * is_not_delegatecall; } else { tempvar authorized = authorized; diff --git a/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.cairo b/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.cairo index 4c0e5562c..c0d88cbdb 100644 --- a/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.cairo +++ b/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.cairo @@ -25,7 +25,6 @@ func test__precompiles_run{ // Given local address; local input_len; - local caller_code_address; local caller_address; local message_address; let (local input) = alloc(); @@ -33,7 +32,6 @@ func test__precompiles_run{ ids.address = program_input["address"] ids.input_len = len(program_input["input"]) segments.write_arg(ids.input, program_input["input"]) - ids.caller_code_address = program_input.get("caller_code_address", 0) ids.caller_address = program_input.get("caller_address", 0) ids.message_address = program_input.get("message_address", 0) %} @@ -43,7 +41,6 @@ func test__precompiles_run{ precompile_address=address, input_len=input_len, input=input, - caller_code_address=caller_code_address, caller_address=caller_address, message_address=message_address, ); diff --git a/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.py b/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.py index 9d040c3b6..eaec65405 100644 --- a/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.py +++ b/cairo_zero/tests/src/kakarot/precompiles/test_precompiles.py @@ -13,8 +13,8 @@ ) from tests.utils.syscall_handler import SyscallHandler -AUTHORIZED_CALLER_CODE = 0xA7071ED -UNAUTHORIZED_CALLER_CODE = 0xC0C0C0 +AUTHORIZED_CALLER_ADDRESS = 0xA7071ED +UNAUTHORIZED_CALLER_ADDRESS = 0xC0C0C0 CALLER_ADDRESS = 0x123ABC432 @@ -80,7 +80,7 @@ def test__p256_verify_precompile( class TestKakarotPrecompiles: @SyscallHandler.patch( "Kakarot_authorized_cairo_precompiles_callers", - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, 1, ) @SyscallHandler.patch("deploy", lambda *_: [0]) @@ -103,38 +103,37 @@ def test_should_deploy_account_when_sender_starknet_address_zero( + f"{0x60:064x}" # data_offset + f"{0x00:064x}" # data_len ), - caller_code_address=AUTHORIZED_CALLER_CODE, - caller_address=CALLER_ADDRESS, + caller_address=AUTHORIZED_CALLER_ADDRESS, message_address=0x75001, ) assert not bool(reverted) assert bytes(return_data) == b"" assert gas_used == CAIRO_PRECOMPILE_GAS - SyscallHandler.mock_deploy.assert_called_once() - return + assert SyscallHandler.mock_deploy.call_count == 1 @SyscallHandler.patch( "Kakarot_authorized_cairo_precompiles_callers", - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, 1, ) @SyscallHandler.patch( "Kakarot_evm_to_starknet_address", CALLER_ADDRESS, 0x1234 ) + @SyscallHandler.patch("deploy", lambda *_: [0]) @pytest.mark.parametrize( - "address, caller_code_address, input_data, expected_return_data, expected_reverted", + "address, caller_address, input_data, expected_return_data, expected_reverted", [ ( 0x75001, - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, bytes.fromhex("0abcdef0"), b"Kakarot: OutOfBoundsRead", True, ), # invalid input ( 0x75001, - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, bytes.fromhex( f"{0xc0de:064x}" + f"{get_selector_from_name('inc'):064x}" @@ -146,7 +145,7 @@ def test_should_deploy_account_when_sender_starknet_address_zero( ), # call_contract ( 0x75001, - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, bytes.fromhex( f"{0xc0de:064x}" + f"{get_selector_from_name('get'):064x}" @@ -159,7 +158,7 @@ def test_should_deploy_account_when_sender_starknet_address_zero( ), # call_contract with return data ( 0x75001, - UNAUTHORIZED_CALLER_CODE, + UNAUTHORIZED_CALLER_ADDRESS, bytes.fromhex("0abcdef0"), b"Kakarot: unauthorizedPrecompile", True, @@ -176,7 +175,7 @@ def test__cairo_precompiles( self, cairo_run, address, - caller_code_address, + caller_address, input_data, expected_return_data, expected_reverted, @@ -197,15 +196,14 @@ def test__cairo_precompiles( "test__precompiles_run", address=address, input=input_data, - caller_code_address=caller_code_address, - caller_address=CALLER_ADDRESS, + caller_address=caller_address, message_address=address, ) assert bool(reverted) == expected_reverted assert bytes(return_data) == expected_return_data assert gas_used == ( CAIRO_PRECOMPILE_GAS - if caller_code_address == AUTHORIZED_CALLER_CODE + if caller_address == AUTHORIZED_CALLER_ADDRESS else 0 ) return @@ -213,7 +211,7 @@ def test__cairo_precompiles( class TestKakarotMessaging: @SyscallHandler.patch( "Kakarot_authorized_cairo_precompiles_callers", - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, 1, ) @SyscallHandler.patch( @@ -221,11 +219,11 @@ class TestKakarotMessaging: 0xC0DE, ) @pytest.mark.parametrize( - "address, caller_code_address, input_data, to_address, expected_reverted_return_data, expected_reverted", + "address, caller_address, input_data, to_address, expected_reverted_return_data, expected_reverted", [ ( 0x75002, - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, encode( ["uint160", "bytes"], [0xC0DE, encode(["uint128"], [0x2A])] ), @@ -235,7 +233,7 @@ class TestKakarotMessaging: ), ( 0x75002, - AUTHORIZED_CALLER_CODE, + AUTHORIZED_CALLER_ADDRESS, encode(["uint160", "bytes"], [0xC0DE, 0x2A.to_bytes(1, "big")]), 0xC0DE, b"", @@ -243,7 +241,7 @@ class TestKakarotMessaging: ), ( 0x75002, - UNAUTHORIZED_CALLER_CODE, + UNAUTHORIZED_CALLER_ADDRESS, bytes.fromhex("0abcdef0"), 0xC0DE, b"Kakarot: unauthorizedPrecompile", @@ -258,7 +256,7 @@ class TestKakarotMessaging: ) def test__cairo_message( self, - caller_code_address, + caller_address, cairo_run, address, input_data, @@ -271,8 +269,7 @@ def test__cairo_message( "test__precompiles_run", address=address, input=input_data, - caller_code_address=caller_code_address, - caller_address=CALLER_ADDRESS, + caller_address=caller_address, message_address=address, ) if expected_reverted: diff --git a/kakarot_scripts/utils/kakarot.py b/kakarot_scripts/utils/kakarot.py index 133d5a05f..09bd80d15 100644 --- a/kakarot_scripts/utils/kakarot.py +++ b/kakarot_scripts/utils/kakarot.py @@ -712,8 +712,7 @@ async def eth_send_transaction( typed_transaction = TypedTransaction.from_dict(payload) evm_tx = EvmAccount.sign_transaction( - typed_transaction.as_dict(), - hex(evm_account.signer.private_key), + typed_transaction.as_dict(), f"{evm_account.signer.private_key:064x}" ) if WEB3.is_connected(): diff --git a/solidity_contracts/src/CairoPrecompiles/CallCairoPrecompileTest.sol b/solidity_contracts/src/CairoPrecompiles/CallCairoPrecompileTest.sol index d1c1c8c3b..2ae3e12fa 100644 --- a/solidity_contracts/src/CairoPrecompiles/CallCairoPrecompileTest.sol +++ b/solidity_contracts/src/CairoPrecompiles/CallCairoPrecompileTest.sol @@ -115,7 +115,7 @@ library Internals { mstore(0x40, add(result, add(0x20, size))) } - require(success, string(abi.encodePacked("CairoLib: call_contract failed with: ", result))); + require(success, string(abi.encodePacked("CairoLib: cairo call failed with: ", result))); return result; } diff --git a/solidity_contracts/src/CairoPrecompiles/DualVmToken.sol b/solidity_contracts/src/CairoPrecompiles/DualVmToken.sol index 3f1075237..aa9a63a02 100644 --- a/solidity_contracts/src/CairoPrecompiles/DualVmToken.sol +++ b/solidity_contracts/src/CairoPrecompiles/DualVmToken.sol @@ -1,16 +1,22 @@ -// SPDX-License-Identifier: AGPL-3.0-only +// SPDX-License-Identifier: MIT pragma solidity 0.8.27; import {WhitelistedCallCairoLib} from "./WhitelistedCallCairoLib.sol"; import {CairoLib} from "kakarot-lib/CairoLib.sol"; +import {NoDelegateCall} from "../NoDelegateCall/NoDelegateCall.sol"; /// @notice EVM adapter into a Cairo ERC20 token /// @dev This implementation is highly experimental /// It relies on CairoLib to perform Cairo precompile calls /// Events are emitted in this contract but also in the Starknet token contract +/// @dev External functions are `NoDelegateCall` to prevent a user making an EVM call to a malicious contract, +/// with any calldata, that would be able to directly control on their behalf any quantity of any one of the ERC20 +/// tokens held by the victim's account contract, with the sole condition that the ERC20 has an +/// authorized DualVmToken wrapper. +/// This is blocked at the protocol level, but made explicit at the contract level /// @author Kakarot /// @author Modified from Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) -contract DualVmToken { +contract DualVmToken is NoDelegateCall { using WhitelistedCallCairoLib for uint256; /*////////////////////////////////////////////////////////////// CAIRO SPECIFIC VARIABLES @@ -204,7 +210,7 @@ contract DualVmToken { //////////////////////////////////////////////////////////////*/ /// @dev Approve an evm account spender for a specific amount - function approve(address spender, uint256 amount) external returns (bool) { + function approve(address spender, uint256 amount) external noDelegateCall returns (bool) { uint256[] memory spenderAddressCalldata = new uint256[](1); spenderAddressCalldata[0] = uint256(uint160(spender)); uint256 spenderStarknetAddress = @@ -220,7 +226,7 @@ contract DualVmToken { /// @param spender The starknet address to approve /// @param amount The amount of tokens to approve /// @return True if the approval was successful - function approve(uint256 spender, uint256 amount) external returns (bool) { + function approve(uint256 spender, uint256 amount) external noDelegateCall returns (bool) { _approve(spender, amount); emit Approval(msg.sender, spender, amount); return true; @@ -245,7 +251,7 @@ contract DualVmToken { /// @param to The evm address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transfer(address to, uint256 amount) external returns (bool) { + function transfer(address to, uint256 amount) external noDelegateCall returns (bool) { uint256[] memory toAddressCalldata = new uint256[](1); toAddressCalldata[0] = uint256(uint160(to)); uint256 toStarknetAddress = @@ -260,7 +266,7 @@ contract DualVmToken { /// @param to The starknet address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transfer(uint256 to, uint256 amount) external returns (bool) { + function transfer(uint256 to, uint256 amount) external noDelegateCall returns (bool) { _transfer(to, amount); emit Transfer(msg.sender, to, amount); return true; @@ -287,7 +293,7 @@ contract DualVmToken { /// @param to The evm address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transferFrom(address from, address to, uint256 amount) external returns (bool) { + function transferFrom(address from, address to, uint256 amount) external noDelegateCall returns (bool) { uint256[] memory toAddressCalldata = new uint256[](1); toAddressCalldata[0] = uint256(uint160(to)); uint256 toStarknetAddress = @@ -309,7 +315,7 @@ contract DualVmToken { /// @param to The evm address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transferFrom(uint256 from, address to, uint256 amount) external returns (bool) { + function transferFrom(uint256 from, address to, uint256 amount) external noDelegateCall returns (bool) { uint256[] memory toAddressCalldata = new uint256[](1); toAddressCalldata[0] = uint256(uint160(to)); uint256 toStarknetAddress = @@ -326,7 +332,7 @@ contract DualVmToken { /// @param to The starknet address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transferFrom(address from, uint256 to, uint256 amount) external returns (bool) { + function transferFrom(address from, uint256 to, uint256 amount) external noDelegateCall returns (bool) { uint256[] memory fromAddressCalldata = new uint256[](1); fromAddressCalldata[0] = uint256(uint160(from)); uint256 fromStarknetAddress = @@ -343,7 +349,7 @@ contract DualVmToken { /// @param to The starknet address to transfer the tokens to /// @param amount The amount of tokens to transfer /// @return True if the transfer was successful - function transferFrom(uint256 from, uint256 to, uint256 amount) external returns (bool) { + function transferFrom(uint256 from, uint256 to, uint256 amount) external noDelegateCall returns (bool) { _transferFrom(from, to, amount); emit Transfer(from, to, amount); return true; diff --git a/solidity_contracts/src/L1L2Messaging/L2KakarotMessaging.sol b/solidity_contracts/src/L1L2Messaging/L2KakarotMessaging.sol index 20a2947f8..becbeebe5 100644 --- a/solidity_contracts/src/L1L2Messaging/L2KakarotMessaging.sol +++ b/solidity_contracts/src/L1L2Messaging/L2KakarotMessaging.sol @@ -2,12 +2,16 @@ pragma solidity 0.8.27; import {CairoLib} from "kakarot-lib/CairoLib.sol"; +import {NoDelegateCall} from "../NoDelegateCall/NoDelegateCall.sol"; -contract L2KakarotMessaging { +contract L2KakarotMessaging is NoDelegateCall { /// @notice Sends a message to a contract on L1. + /// @dev This function is noDelegateCall to prevent attack vectors where a + /// contract can send messages to L1 with arbitrary target addresses and payloads; + /// these messages appear as originated by victim's EVM address. /// @param to The address of the contract on L1 to send the message to. /// @param data The data to send to the contract on L1. - function sendMessageToL1(address to, bytes calldata data) external { + function sendMessageToL1(address to, bytes calldata data) external noDelegateCall { bytes memory payload = abi.encode(to, msg.sender, data); CairoLib.sendMessageToL1(payload); } diff --git a/solidity_contracts/src/NoDelegateCall/DualVmTokenHack.sol b/solidity_contracts/src/NoDelegateCall/DualVmTokenHack.sol new file mode 100644 index 000000000..26a1622fc --- /dev/null +++ b/solidity_contracts/src/NoDelegateCall/DualVmTokenHack.sol @@ -0,0 +1,80 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +contract DualVmTokenHack { + address target; + uint256 constant AMOUNT = 1337; + + constructor(address _target) { + target = _target; + } + + function tryApproveEvm() external returns (bool success) { + (success,) = target.delegatecall{gas: 30000}( + abi.encodeWithSelector(bytes4(keccak256("approve(address,uint256)")), address(this), AMOUNT) + ); + } + + function tryApproveStarknet() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("approve(uint256,uint256)")), uint256(uint160(address(this))), AMOUNT + ) + ); + } + + function tryTransferEvm() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector(bytes4(keccak256("transfer(address,uint256)")), address(this), AMOUNT) + ); + } + + function tryTransferStarknet() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("transfer(uint256,uint256)")), uint256(uint160(address(this))), AMOUNT + ) + ); + } + + function tryTransferFromEvmEvm() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("transferFrom(address,address,uint256)")), msg.sender, address(this), AMOUNT + ) + ); + } + + function tryTransferFromStarknetEvm() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("transferFrom(uint256,address,uint256)")), + uint256(uint160(msg.sender)), + address(this), + AMOUNT + ) + ); + } + + function tryTransferFromEvmStarknet() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("transferFrom(address,uint256,uint256)")), + msg.sender, + uint256(uint160(address(this))), + AMOUNT + ) + ); + } + + function tryTransferFromStarknetStarknet() external returns (bool success) { + (success,) = target.delegatecall( + abi.encodeWithSelector( + bytes4(keccak256("transferFrom(uint256,uint256,uint256)")), + uint256(uint160(msg.sender)), + uint256(uint160(address(this))), + AMOUNT + ) + ); + } +} diff --git a/solidity_contracts/src/NoDelegateCall/DualVmTokenWithoutModifier.sol b/solidity_contracts/src/NoDelegateCall/DualVmTokenWithoutModifier.sol new file mode 100644 index 000000000..197fe368e --- /dev/null +++ b/solidity_contracts/src/NoDelegateCall/DualVmTokenWithoutModifier.sol @@ -0,0 +1,372 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {WhitelistedCallCairoLib} from "../CairoPrecompiles/WhitelistedCallCairoLib.sol"; +import {CairoLib} from "kakarot-lib/CairoLib.sol"; + +/// @notice EVM adapter into a Cairo ERC20 token +/// @dev This implementation is highly experimental +/// It relies on CairoLib to perform Cairo precompile calls +/// Events are emitted in this contract but also in the Starknet token contract +/// @dev External functions are to prevent a user making an EVM call to a malicious contract, +/// with any calldata, that would be able to directly control on their behalf any quantity of any one of the ERC20 +/// tokens held by the victim's account contract, with the sole condition that the ERC20 has an +/// authorized DualVmToken wrapper. +/// @author Kakarot +/// @author Modified from Solmate (https://github.com/transmissions11/solmate/blob/main/src/tokens/ERC20.sol) +contract DualVmTokenWithoutModifier { + using WhitelistedCallCairoLib for uint256; + /*////////////////////////////////////////////////////////////// + CAIRO SPECIFIC VARIABLES + //////////////////////////////////////////////////////////////*/ + + /// @dev The prime number used in the Starknet field + uint256 public constant STARKNET_FIELD_PRIME = 2 ** 251 + 17 * 2 ** 192 + 1; + + /// @dev The address of the starknet token to call + uint256 public immutable starknetToken; + + /// @dev The address of the kakarot starknet contract to call + uint256 public immutable kakarot; + + /*////////////////////////////////////////////////////////////// + EVENTS + //////////////////////////////////////////////////////////////*/ + + /// @dev Emitted when tokens are transferred from one address to another + event Transfer(address indexed from, address indexed to, uint256 amount); + + /// @dev Emitted when tokens are transferred from one starknet address to an evm address + event Transfer(uint256 indexed from, address indexed to, uint256 amount); + + /// @dev Emitted when tokens are transferred from one address to a starknet address + event Transfer(address indexed from, uint256 indexed to, uint256 amount); + + /// @dev Emitted when tokens are transferred from one starknet address to another + event Transfer(uint256 indexed from, uint256 indexed to, uint256 amount); + + /// @dev Emitted when the allowance of a spender over the owner's tokens is set + event Approval(address indexed owner, address indexed spender, uint256 amount); + + /// @dev Emitted when the allowance of a starknet address spender over the owner's tokens is set + event Approval(address indexed owner, uint256 indexed spender, uint256 amount); + + /*////////////////////////////////////////////////////////////// + ERRORS + //////////////////////////////////////////////////////////////*/ + + /// @dev Emitted when an invalid starknet address is used + error InvalidStarknetAddress(); + + /*////////////////////////////////////////////////////////////// + METADATA ACCESS + //////////////////////////////////////////////////////////////*/ + + function name() external view returns (string memory) { + bytes memory returnData = starknetToken.staticcallCairo("name"); + + // Legacy tokens return a felt for name instead of a ByteArray + if (returnData.length == 32) { + return string(returnData); + } + + return CairoLib.byteArrayToString(returnData); + } + + function symbol() external view returns (string memory) { + bytes memory returnData = starknetToken.staticcallCairo("symbol"); + + // Legacy tokens return a felt for name instead of a ByteArray + if (returnData.length == 32) { + return string(returnData); + } + + return CairoLib.byteArrayToString(returnData); + } + + function decimals() external view returns (uint8) { + bytes memory returnData = starknetToken.staticcallCairo("decimals"); + return abi.decode(returnData, (uint8)); + } + + /*////////////////////////////////////////////////////////////// + ERC20 STORAGE + //////////////////////////////////////////////////////////////*/ + + function totalSupply() external view returns (uint256) { + bytes memory returnData = starknetToken.staticcallCairo("total_supply"); + (uint128 valueLow, uint128 valueHigh) = abi.decode(returnData, (uint128, uint128)); + return uint256(valueLow) + (uint256(valueHigh) << 128); + } + + /// @dev This function is used to get the balance of an evm account + /// @param account The evm account to get the balance of + /// @return The balance of the evm address + function balanceOf(address account) external view returns (uint256) { + uint256[] memory kakarotCallData = new uint256[](1); + kakarotCallData[0] = uint256(uint160(account)); + uint256 accountStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", kakarotCallData), (uint256)); + return _balanceOf(accountStarknetAddress); + } + + /// @dev This function is used to get the balance of a starknet address + /// @param starknetAddress The starknet address to get the balance of + /// @return The balance of the starknet address + function balanceOf(uint256 starknetAddress) external view returns (uint256) { + return _balanceOf(starknetAddress); + } + + function _balanceOf(uint256 starknetAddress) private view returns (uint256) { + if (starknetAddress >= STARKNET_FIELD_PRIME) { + revert InvalidStarknetAddress(); + } + uint256[] memory balanceOfCallData = new uint256[](1); + balanceOfCallData[0] = starknetAddress; + bytes memory returnData = starknetToken.staticcallCairo("balance_of", balanceOfCallData); + (uint128 valueLow, uint128 valueHigh) = abi.decode(returnData, (uint128, uint128)); + return uint256(valueLow) + (uint256(valueHigh) << 128); + } + + /// @dev Get the allowance of a spender over the owner's tokens + /// @param owner The evm address of the owner of the tokens + /// @param spender The evm address of the spender to get the allowance of + /// @return The allowance of spender over the owner's tokens + function allowance(address owner, address spender) external view returns (uint256) { + uint256[] memory ownerAddressCalldata = new uint256[](1); + ownerAddressCalldata[0] = uint256(uint160(owner)); + uint256 ownerStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", ownerAddressCalldata), (uint256)); + + uint256[] memory spenderAddressCalldata = new uint256[](1); + spenderAddressCalldata[0] = uint256(uint160(spender)); + uint256 spenderStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", spenderAddressCalldata), (uint256)); + + return _allowance(ownerStarknetAddress, spenderStarknetAddress); + } + + /// @dev Get the allowance of the spender when it is a starknet address + /// @param owner The evm address of the owner of the tokens + /// @param spender The starknet address of the spender to get the allowance of + /// @return The allowance of spender over the owner's tokens + function allowance(address owner, uint256 spender) external view returns (uint256) { + uint256[] memory ownerAddressCalldata = new uint256[](1); + ownerAddressCalldata[0] = uint256(uint160(owner)); + uint256 ownerStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", ownerAddressCalldata), (uint256)); + + return _allowance(ownerStarknetAddress, spender); + } + + /// @dev Get the allowance of spender when the owner is a starknet address + /// @param owner The starknet address of the owner of the tokens + /// @param spender The evm address of the spender to get the allowance of + /// @return The allowance of spender over the owner's tokens + function allowance(uint256 owner, address spender) external view returns (uint256) { + uint256[] memory spenderAddressCalldata = new uint256[](1); + spenderAddressCalldata[0] = uint256(uint160(spender)); + uint256 spenderStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", spenderAddressCalldata), (uint256)); + + return _allowance(owner, spenderStarknetAddress); + } + + /// @dev Get the allowance of spender when both the owner and spender are starknet addresses + /// @param owner The starknet address of the owner of the tokens + /// @param spender The starknet address of the spender to get the allowance of + /// @return The allowance of spender over the owner's tokens + function allowance(uint256 owner, uint256 spender) external view returns (uint256) { + return _allowance(owner, spender); + } + + function _allowance(uint256 owner, uint256 spender) private view returns (uint256) { + if (owner >= STARKNET_FIELD_PRIME || spender >= STARKNET_FIELD_PRIME) { + revert InvalidStarknetAddress(); + } + uint256[] memory allowanceCallData = new uint256[](2); + allowanceCallData[0] = owner; + allowanceCallData[1] = spender; + + bytes memory returnData = starknetToken.staticcallCairo("allowance", allowanceCallData); + (uint128 valueLow, uint128 valueHigh) = abi.decode(returnData, (uint128, uint128)); + + return uint256(valueLow) + (uint256(valueHigh) << 128); + } + + /*////////////////////////////////////////////////////////////// + CONSTRUCTOR + //////////////////////////////////////////////////////////////*/ + + constructor(uint256 _kakarot, uint256 _starknetToken) { + kakarot = _kakarot; + starknetToken = _starknetToken; + } + + /*////////////////////////////////////////////////////////////// + ERC20 LOGIC + //////////////////////////////////////////////////////////////*/ + + /// @dev Approve an evm account spender for a specific amount + function approve(address spender, uint256 amount) external returns (bool) { + uint256[] memory spenderAddressCalldata = new uint256[](1); + spenderAddressCalldata[0] = uint256(uint160(spender)); + uint256 spenderStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", spenderAddressCalldata), (uint256)); + + _approve(spenderStarknetAddress, amount); + + emit Approval(msg.sender, spender, amount); + return true; + } + + /// @dev Approve a starknet address for a specific amount + /// @param spender The starknet address to approve + /// @param amount The amount of tokens to approve + /// @return True if the approval was successful + function approve(uint256 spender, uint256 amount) external returns (bool) { + _approve(spender, amount); + emit Approval(msg.sender, spender, amount); + return true; + } + + function _approve(uint256 spender, uint256 amount) private { + if (spender >= STARKNET_FIELD_PRIME) { + revert InvalidStarknetAddress(); + } + // Split amount in [low, high] + uint128 amountLow = uint128(amount); + uint128 amountHigh = uint128(amount >> 128); + uint256[] memory approveCallData = new uint256[](3); + approveCallData[0] = spender; + approveCallData[1] = uint256(amountLow); + approveCallData[2] = uint256(amountHigh); + + starknetToken.delegatecallCairo("approve", approveCallData); + } + + /// @dev Transfer tokens to an evm account + /// @param to The evm address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transfer(address to, uint256 amount) external returns (bool) { + uint256[] memory toAddressCalldata = new uint256[](1); + toAddressCalldata[0] = uint256(uint160(to)); + uint256 toStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", toAddressCalldata), (uint256)); + + _transfer(toStarknetAddress, amount); + emit Transfer(msg.sender, to, amount); + return true; + } + + /// @dev Transfer tokens to a starknet address + /// @param to The starknet address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transfer(uint256 to, uint256 amount) external returns (bool) { + _transfer(to, amount); + emit Transfer(msg.sender, to, amount); + return true; + } + + function _transfer(uint256 to, uint256 amount) private { + if (to >= STARKNET_FIELD_PRIME) { + revert InvalidStarknetAddress(); + } + // Split amount in [low, high] + uint128 amountLow = uint128(amount); + uint128 amountHigh = uint128(amount >> 128); + + uint256[] memory transferCallData = new uint256[](3); + transferCallData[0] = to; + transferCallData[1] = uint256(amountLow); + transferCallData[2] = uint256(amountHigh); + + starknetToken.delegatecallCairo("transfer", transferCallData); + } + + /// @dev Transfer tokens from one evm address to another + /// @param from The evm address to transfer the tokens from + /// @param to The evm address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transferFrom(address from, address to, uint256 amount) external returns (bool) { + uint256[] memory toAddressCalldata = new uint256[](1); + toAddressCalldata[0] = uint256(uint160(to)); + uint256 toStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", toAddressCalldata), (uint256)); + + uint256[] memory fromAddressCalldata = new uint256[](1); + fromAddressCalldata[0] = uint256(uint160(from)); + uint256 fromStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", fromAddressCalldata), (uint256)); + + _transferFrom(fromStarknetAddress, toStarknetAddress, amount); + + emit Transfer(from, to, amount); + return true; + } + + /// @dev Transfer tokens from a starknet address to an evm address + /// @param from The starknet address to transfer the tokens from + /// @param to The evm address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transferFrom(uint256 from, address to, uint256 amount) external returns (bool) { + uint256[] memory toAddressCalldata = new uint256[](1); + toAddressCalldata[0] = uint256(uint160(to)); + uint256 toStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", toAddressCalldata), (uint256)); + + _transferFrom(from, toStarknetAddress, amount); + + emit Transfer(from, to, amount); + return true; + } + + /// @dev Transfer from tokens to a starknet address + /// @param from The evm address to transfer the tokens from + /// @param to The starknet address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transferFrom(address from, uint256 to, uint256 amount) external returns (bool) { + uint256[] memory fromAddressCalldata = new uint256[](1); + fromAddressCalldata[0] = uint256(uint160(from)); + uint256 fromStarknetAddress = + abi.decode(kakarot.staticcallCairo("get_starknet_address", fromAddressCalldata), (uint256)); + + _transferFrom(fromStarknetAddress, to, amount); + + emit Transfer(from, to, amount); + return true; + } + + /// @dev Transfer from tokens when both from and to address as starknet addresses + /// @param from The starknet address to transfer the tokens from + /// @param to The starknet address to transfer the tokens to + /// @param amount The amount of tokens to transfer + /// @return True if the transfer was successful + function transferFrom(uint256 from, uint256 to, uint256 amount) external returns (bool) { + _transferFrom(from, to, amount); + emit Transfer(from, to, amount); + return true; + } + + function _transferFrom(uint256 from, uint256 to, uint256 amount) private { + if (from >= STARKNET_FIELD_PRIME || to >= STARKNET_FIELD_PRIME) { + revert InvalidStarknetAddress(); + } + // Split amount in [low, high] + uint128 amountLow = uint128(amount); + uint128 amountHigh = uint128(amount >> 128); + + uint256[] memory transferFromCallData = new uint256[](4); + transferFromCallData[0] = from; + transferFromCallData[1] = to; + transferFromCallData[2] = uint256(amountLow); + transferFromCallData[3] = uint256(amountHigh); + + starknetToken.delegatecallCairo("transfer_from", transferFromCallData); + } +} diff --git a/solidity_contracts/src/NoDelegateCall/L2KakarotMessagingHack.sol b/solidity_contracts/src/NoDelegateCall/L2KakarotMessagingHack.sol new file mode 100644 index 000000000..e5b4200b9 --- /dev/null +++ b/solidity_contracts/src/NoDelegateCall/L2KakarotMessagingHack.sol @@ -0,0 +1,18 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.27; + +import {L2KakarotMessaging} from "../L1L2Messaging/L2KakarotMessaging.sol"; + +contract L2MessagingHack { + L2KakarotMessaging public immutable target; + + constructor(address _target) { + target = L2KakarotMessaging(_target); + } + + function trySendMessageToL1(address to, bytes calldata data) external returns (bool success) { + // Try to send message through delegatecall + (success,) = + address(target).delegatecall(abi.encodeWithSelector(L2KakarotMessaging.sendMessageToL1.selector, to, data)); + } +} diff --git a/solidity_contracts/src/NoDelegateCall/NoDelegateCall.sol b/solidity_contracts/src/NoDelegateCall/NoDelegateCall.sol new file mode 100644 index 000000000..31032d30a --- /dev/null +++ b/solidity_contracts/src/NoDelegateCall/NoDelegateCall.sol @@ -0,0 +1,28 @@ +// SPDX-License-Identifier: BUSL-1.1 +// Author: Uniswap Labs https://github.com/Uniswap/v3-core/blob/main/contracts/NoDelegateCall.sol +pragma solidity 0.8.27; + +/// @title Prevents delegatecall to a contract +/// @notice Base contract that provides a modifier for preventing delegatecall to methods in a child contract +abstract contract NoDelegateCall { + /// @dev The original address of this contract + address private immutable original; + + constructor() { + // Immutables are computed in the init code of the contract, and then inlined into the deployed bytecode. + // In other words, this variable won't change when it's checked at runtime. + original = address(this); + } + + /// @dev Private method is used instead of inlining into modifier because modifiers are copied into each method, + /// and the use of immutable means the address bytes are copied in every place the modifier is used. + function checkNotDelegateCall() private view { + require(address(this) == original); + } + + /// @notice Prevents delegatecall into the modified method + modifier noDelegateCall() { + checkNotDelegateCall(); + _; + } +} diff --git a/tests/end_to_end/CairoPrecompiles/test_call_cairo_precompile.py b/tests/end_to_end/CairoPrecompiles/test_call_cairo_precompile.py index 640bd79cf..4cdba73e0 100644 --- a/tests/end_to_end/CairoPrecompiles/test_call_cairo_precompile.py +++ b/tests/end_to_end/CairoPrecompiles/test_call_cairo_precompile.py @@ -5,7 +5,7 @@ from kakarot_scripts.constants import NETWORK from kakarot_scripts.utils.kakarot import deploy, eth_send_transaction from kakarot_scripts.utils.starknet import get_contract, invoke -from tests.utils.errors import cairo_error +from tests.utils.errors import cairo_error, evm_error CALL_CAIRO_PRECOMPILE = 0x75004 @@ -93,17 +93,13 @@ async def test_should_increase_counter_from_solidity( async def test_should_fail_when_called_with_delegatecall( self, cairo_counter_caller ): - with cairo_error( - "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles" - ): + with evm_error("CairoLib: cairo call failed with"): await cairo_counter_caller.incrementCairoCounterDelegatecall() async def test_should_fail_when_called_with_callcode( self, cairo_counter_caller ): - with cairo_error( - "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles" - ): + with evm_error("CairoLib: cairo call failed with"): await cairo_counter_caller.incrementCairoCounterCallcode() @pytest.mark.skipif( diff --git a/tests/end_to_end/CairoPrecompiles/test_multicall_cairo_precompile.py b/tests/end_to_end/CairoPrecompiles/test_multicall_cairo_precompile.py index b1d2c6611..51905a5fb 100644 --- a/tests/end_to_end/CairoPrecompiles/test_multicall_cairo_precompile.py +++ b/tests/end_to_end/CairoPrecompiles/test_multicall_cairo_precompile.py @@ -8,7 +8,7 @@ from kakarot_scripts.utils.kakarot import deploy, eth_send_transaction from kakarot_scripts.utils.starknet import get_contract, invoke -from tests.utils.errors import cairo_error +from tests.utils.errors import evm_error @pytest_asyncio.fixture(scope="module") @@ -116,15 +116,11 @@ async def test_should_increase_counter_in_multicall_from_solidity( async def test_should_fail_when_called_with_delegatecall( self, multicall_cairo_counter_caller ): - with cairo_error( - "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles" - ): + with evm_error("CairoLib: call_contract failed with"): await multicall_cairo_counter_caller.incrementCairoCounterDelegatecall() async def test_should_fail_when_called_with_callcode( self, multicall_cairo_counter_caller ): - with cairo_error( - "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles" - ): + with evm_error("CairoLib: call_contract failed with"): await multicall_cairo_counter_caller.incrementCairoCounterCallcode() diff --git a/tests/end_to_end/CairoPrecompiles/test_whitelisted_call_cairo_precompile.py b/tests/end_to_end/CairoPrecompiles/test_whitelisted_call_cairo_precompile.py index 10cda7053..a9b945054 100644 --- a/tests/end_to_end/CairoPrecompiles/test_whitelisted_call_cairo_precompile.py +++ b/tests/end_to_end/CairoPrecompiles/test_whitelisted_call_cairo_precompile.py @@ -3,7 +3,7 @@ from kakarot_scripts.utils.kakarot import deploy, get_eoa from kakarot_scripts.utils.starknet import get_contract, invoke, wait_for_transaction -from tests.utils.errors import cairo_error +from tests.utils.errors import cairo_error, evm_error @pytest_asyncio.fixture(scope="module") @@ -82,9 +82,7 @@ async def test_should_fail_precompile_caller_not_whitelisted( "WhitelistedCallCairoPrecompileTest", cairo_counter.address, ) - with cairo_error( - "EVM tx reverted, reverting SN tx because of previous calls to cairo precompiles" - ): + with evm_error("CairoLib: cairo call failed with:"): await cairo_counter_caller.incrementCairoCounter() async def test_last_caller_address_should_be_eoa(self, cairo_counter_caller): diff --git a/tests/end_to_end/NoDelegateCall/test_dual_vm_token_hack.py b/tests/end_to_end/NoDelegateCall/test_dual_vm_token_hack.py new file mode 100644 index 000000000..2761c2f81 --- /dev/null +++ b/tests/end_to_end/NoDelegateCall/test_dual_vm_token_hack.py @@ -0,0 +1,174 @@ +import logging + +import pytest +import pytest_asyncio + +from kakarot_scripts.utils.kakarot import deploy as deploy_kakarot +from kakarot_scripts.utils.starknet import deploy as deploy_starknet +from kakarot_scripts.utils.starknet import get_contract as get_contract_starknet +from kakarot_scripts.utils.starknet import invoke + +logging.basicConfig() +logger = logging.getLogger(__name__) +logger.setLevel(logging.INFO) + + +@pytest_asyncio.fixture(scope="package") +async def starknet_token(owner): + address = await deploy_starknet( + "StarknetToken", + "MyToken", + "MTK", + 18, + int(2**256 - 1), + owner.starknet_contract.address, + ) + return get_contract_starknet("StarknetToken", address=address) + + +@pytest_asyncio.fixture( + scope="package", + params=[ + ("CairoPrecompiles", "DualVmToken"), + ("NoDelegateCallTesting", "DualVmTokenWithoutModifier"), + ], + ids=["Modifier", "NoModifier"], +) +async def dual_vm_token(request, kakarot, starknet_token, owner): + dual_vm_token = await deploy_kakarot( + request.param[0], + request.param[1], + kakarot.address, + starknet_token.address, + caller_eoa=owner.starknet_contract, + ) + + await invoke( + "kakarot", + "set_authorized_cairo_precompile_caller", + int(dual_vm_token.address, 16), + True, + ) + return dual_vm_token + + +@pytest_asyncio.fixture(scope="package") +async def hack_vm_token(dual_vm_token, owner): + hack_vm_token = await deploy_kakarot( + "CairoPrecompiles", + "DualVmTokenHack", + dual_vm_token.address, + caller_eoa=owner.starknet_contract, + ) + return hack_vm_token + + +@pytest.mark.asyncio(scope="module") +@pytest.mark.CairoPrecompiles +class TestDualVmToken: + class TestActions: + async def test_malicious_approve_address_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token, owner + ): + result = await hack_vm_token.functions["tryApproveEvm()"](gas_limit=1000000) + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + allowance = await dual_vm_token.functions["allowance(address,address)"]( + owner.address, hack_vm_token.address + ) + assert allowance == 0 + + async def test_malicious_approve_starknet_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token, owner + ): + result = await hack_vm_token.functions["tryApproveStarknet()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + allowance = await dual_vm_token.functions["allowance(address,uint256)"]( + owner.address, int(hack_vm_token.address, 16) + ) + assert allowance == 0 + + async def test_malicious_transfer_address_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions["tryTransferEvm()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(address)"]( + hack_vm_token.address + ) + assert balance == 0 + + async def test_malicious_transfer_starknet_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions["tryTransferStarknet()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(uint256)"]( + int(hack_vm_token.address, 16) + ) + assert balance == 0 + + async def test_malicious_transfer_from_address_address_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions["tryTransferFromEvmEvm()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(address)"]( + hack_vm_token.address + ) + assert balance == 0 + + async def test_malicious_transfer_from_starknet_address_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions["tryTransferFromStarknetEvm()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(address)"]( + hack_vm_token.address + ) + assert balance == 0 + + async def test_malicious_transfer_from_address_starknet_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions["tryTransferFromEvmStarknet()"]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(uint256)"]( + int(hack_vm_token.address, 16) + ) + assert balance == 0 + + async def test_malicious_transfer_from_starknet_starknet_should_fail_nodelegatecall( + self, dual_vm_token, hack_vm_token + ): + result = await hack_vm_token.functions[ + "tryTransferFromStarknetStarknet()" + ]() + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 + + balance = await dual_vm_token.functions["balanceOf(uint256)"]( + int(hack_vm_token.address, 16) + ) + assert balance == 0 diff --git a/tests/end_to_end/NoDelegateCall/test_l2_messaging_hack.py b/tests/end_to_end/NoDelegateCall/test_l2_messaging_hack.py new file mode 100644 index 000000000..a73eb6215 --- /dev/null +++ b/tests/end_to_end/NoDelegateCall/test_l2_messaging_hack.py @@ -0,0 +1,35 @@ +import pytest +import pytest_asyncio +from eth_utils.address import to_checksum_address + +from kakarot_scripts.utils.kakarot import deploy +from kakarot_scripts.utils.kakarot import get_deployments as get_evm_deployments + + +@pytest_asyncio.fixture(scope="function") +async def messaging_hack_contract(owner): + return await deploy( + "NoDelegateCallTesting", + "L2MessagingHack", + _target=to_checksum_address( + get_evm_deployments()["L2KakarotMessaging"]["address"] + ), + caller_eoa=owner.starknet_contract, + ) + + +@pytest.mark.asyncio(scope="module") +class TestL2MessagingHack: + async def test_malicious_message_should_fail_nodelegatecall( + self, + messaging_hack_contract, + ): + malicious_target = "0x1234567890123456789012345678901234567890" + malicious_data = "0xdeadbeef" + + result = await messaging_hack_contract.functions[ + "trySendMessageToL1(address,bytes)" + ](malicious_target, malicious_data) + assert result["success"] == 1 + underlying_call_succeeded = int.from_bytes(bytes(result["response"]), "big") + assert underlying_call_succeeded == 0 diff --git a/tests/end_to_end/conftest.py b/tests/end_to_end/conftest.py index a4fe21b1a..576141f87 100644 --- a/tests/end_to_end/conftest.py +++ b/tests/end_to_end/conftest.py @@ -90,7 +90,7 @@ async def _factory(amount=0): address=get_deployments()["Coinbase"]["address"], ) gas_price = (await call("kakarot", "get_base_fee")).base_fee - gas_limit = 40_000 + gas_limit = 100_000 tx_cost = gas_limit * gas_price for wallet in deployed: balance = await eth_balance_of(wallet.address)