fix: add NoDelegateCall modifiers to DualVMToken

solidity_contracts/src/CairoPrecompiles/DualVmToken.sol

@@ -1,16 +1,21 @@
-// 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 "../Security/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.
 /// @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
@@ -192,7 +197,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 =
@@ -208,7 +213,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;
@@ -233,7 +238,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 =
@@ -248,7 +253,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;
@@ -275,7 +280,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 =
@@ -297,7 +302,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 =
@@ -314,7 +319,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 =
@@ -331,7 +336,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;

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 "../Security/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);
     }

solidity_contracts/src/Security/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(
+            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
+            )
+        );
+    }
+}

solidity_contracts/src/Security/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));
+    }
+}

solidity_contracts/src/Security/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();
+        _;
+    }
+}

tests/end_to_end/Security/test_dual_vm_token_hack.py

@@ -0,0 +1,154 @@
+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="function")
+async def starknet_token(owner):
+    address = await deploy_starknet(
+        "StarknetToken", int(1e18), owner.starknet_contract.address
+    )
+    return get_contract_starknet("StarknetToken", address=address)
+
+
+@pytest_asyncio.fixture(scope="function")
+async def dual_vm_token(kakarot, starknet_token, owner):
+    dual_vm_token = await deploy_kakarot(
+        "CairoPrecompiles",
+        "DualVmToken",
+        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="function")
+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_nodelegatecaltest_malicious_approve_address_should_fail_nodelegatecall(
+            self, dual_vm_token, hack_vm_token, owner
+        ):
+            result = await hack_vm_token.functions["tryApproveEvm()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions["tryTransferEvm()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions["tryTransferStarknet()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions["tryTransferFromEvmEvm()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions["tryTransferFromStarknetEvm()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions["tryTransferFromEvmStarknet()"]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert 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, owner
+        ):
+            result = await hack_vm_token.functions[
+                "tryTransferFromStarknetStarknet()"
+            ]()
+            call_succeeded = int.from_bytes(bytes(result["response"]), "big")
+            assert call_succeeded == 0
+
+            balance = await dual_vm_token.functions["balanceOf(uint256)"](
+                int(hack_vm_token.address, 16)
+            )
+            assert balance == 0