Skip to content

Commit

Permalink
fix: add NoDelegateCall modifiers to DualVMToken
Browse files Browse the repository at this point in the history
  • Loading branch information
enitrat committed Nov 4, 2024
1 parent 171db61 commit 6fe6ce4
Show file tree
Hide file tree
Showing 8 changed files with 337 additions and 46 deletions.
36 changes: 2 additions & 34 deletions kakarot_scripts/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,41 +91,9 @@ class NetworkType(Enum):
"max_wait": 3,
"relayers": [
{
"address": 0xE29882A1FCBA1E7E10CAD46212257FEA5C752A4F9B1B1EC683C503A2CF5C8A,
"private_key": 0x14D6672DCB4B77CA36A887E9A11CD9D637D5012468175829E9C6E770C61642,
},
{
"address": 0x29873C310FBEFDE666DC32A1554FEA6BB45EECC84F680F8A2B0A8FBB8CB89AF,
"private_key": 0xC5B2FCAB997346F3EA1C00B002ECF6F382C5F9C9659A3894EB783C5320F912,
},
{
"address": 0x2D71E9C974539BB3FFB4B115E66A23D0F62A641EA66C4016E903454C8753BBC,
"private_key": 0x33003003001800009900180300D206308B0070DB00121318D17B5E6262150B,
},
{
"address": 0x3EBB4767AAE1262F8EB28D9368DB5388CFE367F50552A8244123506F0B0BCCA,
"private_key": 0x3E3979C1ED728490308054FE357A9F49CF67F80F9721F44CC57235129E090F4,
},
{
"address": 0x541DA8F7F3AB8247329D22B3987D1FFB181BC8DC7F9611A6ECCEC3B0749A585,
"private_key": 0x736ADBBCDAC7CC600F89051DB1ABBC16B9996B46F6B58A9752A11C1028A8EC8,
},
{
"address": 0x56C155B624FDF6BFC94F7B37CF1DBEBB5E186EF2E4AB2762367CD07C8F892A1,
"private_key": 0x6BF3604BCB41FED6C42BCCA5436EEB65083A982FF65DB0DC123F65358008B51,
},
{
"address": 0x6162896D1D7AB204C7CCAC6DD5F8E9E7C25ECD5AE4FCB4AD32E57786BB46E03,
"private_key": 0x1800000000300000180000000000030000000000003006001800006600,
},
{
"address": 0x66EFB28AC62686966AE85095FF3A772E014E7FBF56D4C5F6FAC5606D4DDE23A,
"private_key": 0x283D1E73776CD4AC1AC5F0B879F561BDED25ECEB2CC589C674AF0CEC41DF441,
},
{
"address": 0x6B86E40118F29EBE393A75469B4D926C7A44C2E2681B6D319520B7C1156D114,
"address": 0x13D9EE239F33FEA4F8785B9E3870ADE909E20A9599AE7CD62C1C292B73AF1B7,
"private_key": 0x1C9053C053EDF324AEC366A34C6901B1095B07AF69495BFFEC7D7FE21EFFB1B,
},
}
],
},
"madara": {
Expand Down
25 changes: 15 additions & 10 deletions solidity_contracts/src/CairoPrecompiles/DualVmToken.sol
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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 =
Expand All @@ -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;
Expand All @@ -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 =
Expand All @@ -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;
Expand All @@ -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 =
Expand All @@ -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 =
Expand All @@ -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 =
Expand All @@ -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;
Expand Down
8 changes: 6 additions & 2 deletions solidity_contracts/src/L1L2Messaging/L2KakarotMessaging.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
80 changes: 80 additions & 0 deletions solidity_contracts/src/Security/DualVmTokenHack.sol
Original file line number Diff line number Diff line change
@@ -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
)
);
}
}
18 changes: 18 additions & 0 deletions solidity_contracts/src/Security/L2KakarotMessagingHack.sol
Original file line number Diff line number Diff line change
@@ -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));
}
}
28 changes: 28 additions & 0 deletions solidity_contracts/src/Security/NoDelegateCall.sol
Original file line number Diff line number Diff line change
@@ -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();
_;
}
}
Loading

0 comments on commit 6fe6ce4

Please sign in to comment.