From 393f8a3b7476b56ed1779b30642d38ec52c7982b Mon Sep 17 00:00:00 2001 From: skosito Date: Fri, 4 Oct 2024 18:43:42 +0200 Subject: [PATCH] back port msg and payload size fix --- v2/contracts/evm/GatewayEVM.sol | 6 ++++ v2/contracts/evm/interfaces/IGatewayEVM.sol | 3 ++ v2/contracts/zevm/GatewayZEVM.sol | 7 ++++- v2/contracts/zevm/interfaces/IGatewayZEVM.sol | 6 ++-- v2/test/GatewayEVM.t.sol | 28 +++++++++++++++++++ v2/test/GatewayZEVM.t.sol | 22 +++++++++++++++ 6 files changed, 68 insertions(+), 4 deletions(-) diff --git a/v2/contracts/evm/GatewayEVM.sol b/v2/contracts/evm/GatewayEVM.sol index 8211d219..bed6a39e 100644 --- a/v2/contracts/evm/GatewayEVM.sol +++ b/v2/contracts/evm/GatewayEVM.sol @@ -42,6 +42,8 @@ contract GatewayEVM is bytes32 public constant ASSET_HANDLER_ROLE = keccak256("ASSET_HANDLER_ROLE"); /// @notice New role identifier for pauser role. bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); + /// @notice Max size of payload + revertOptions revert message. + uint256 public constant MAX_PAYLOAD_SIZE = 1024; /// @custom:oz-upgrades-unsafe-allow constructor constructor() { @@ -280,6 +282,7 @@ contract GatewayEVM is { if (msg.value == 0) revert InsufficientETHAmount(); if (receiver == address(0)) revert ZeroAddress(); + if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded(); (bool deposited,) = tssAddress.call{ value: msg.value }(""); @@ -307,6 +310,7 @@ contract GatewayEVM is { if (amount == 0) revert InsufficientERC20Amount(); if (receiver == address(0)) revert ZeroAddress(); + if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded(); transferFromToAssetHandler(msg.sender, asset, amount); @@ -327,6 +331,8 @@ contract GatewayEVM is nonReentrant { if (receiver == address(0)) revert ZeroAddress(); + if (payload.length + revertOptions.revertMessage.length >= MAX_PAYLOAD_SIZE) revert PayloadSizeExceeded(); + emit Called(msg.sender, receiver, payload, revertOptions); } diff --git a/v2/contracts/evm/interfaces/IGatewayEVM.sol b/v2/contracts/evm/interfaces/IGatewayEVM.sol index f5a13c6b..adf976a6 100644 --- a/v2/contracts/evm/interfaces/IGatewayEVM.sol +++ b/v2/contracts/evm/interfaces/IGatewayEVM.sol @@ -87,6 +87,9 @@ interface IGatewayEVMErrors { /// @notice Error when trying to call onRevert method using arbitrary call. error NotAllowedToCallOnRevert(); + + /// @notice Error indicating payload size exceeded in external functions. + error PayloadSizeExceeded(); } /// @title IGatewayEVM diff --git a/v2/contracts/zevm/GatewayZEVM.sol b/v2/contracts/zevm/GatewayZEVM.sol index 4a69c4c0..928e05af 100644 --- a/v2/contracts/zevm/GatewayZEVM.sol +++ b/v2/contracts/zevm/GatewayZEVM.sol @@ -36,6 +36,9 @@ contract GatewayZEVM is /// @notice New role identifier for pauser role. bytes32 public constant PAUSER_ROLE = keccak256("PAUSER_ROLE"); + /// @notice Max size of message + revertOptions revert message. + uint256 public constant MAX_MESSAGE_SIZE = 1024; + /// @dev Only Fungible module address allowed modifier. modifier onlyFungible() { if (msg.sender != FUNGIBLE_MODULE_ADDRESS) { @@ -178,6 +181,7 @@ contract GatewayZEVM is if (receiver.length == 0) revert ZeroAddress(); if (amount == 0) revert InsufficientZRC20Amount(); if (gasLimit == 0) revert InsufficientGasLimit(); + if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded(); uint256 gasFee = _withdrawZRC20WithGasLimit(amount, zrc20, gasLimit); emit Withdrawn( @@ -234,6 +238,7 @@ contract GatewayZEVM is { if (receiver.length == 0) revert ZeroAddress(); if (amount == 0) revert InsufficientZetaAmount(); + if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded(); _transferZETA(amount, FUNGIBLE_MODULE_ADDRESS); emit Withdrawn(msg.sender, chainId, receiver, address(zetaToken), amount, 0, 0, message, 0, revertOptions); @@ -257,8 +262,8 @@ contract GatewayZEVM is whenNotPaused { if (receiver.length == 0) revert ZeroAddress(); - if (message.length == 0) revert EmptyMessage(); if (gasLimit == 0) revert InsufficientGasLimit(); + if (message.length + revertOptions.revertMessage.length >= MAX_MESSAGE_SIZE) revert MessageSizeExceeded(); (address gasZRC20, uint256 gasFee) = IZRC20(zrc20).withdrawGasFeeWithGasLimit(gasLimit); if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) { diff --git a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol index 31df83e5..b9d8b2ee 100644 --- a/v2/contracts/zevm/interfaces/IGatewayZEVM.sol +++ b/v2/contracts/zevm/interfaces/IGatewayZEVM.sol @@ -84,11 +84,11 @@ interface IGatewayZEVMErrors { /// @notice Error indicating that only WZETA or the Fungible module can call the function. error OnlyWZETAOrFungible(); - /// @notice Error indicating call method received empty message as argument. - error EmptyMessage(); - /// @notice Error indicating an insufficient gas limit. error InsufficientGasLimit(); + + /// @notice Error indicating message size exceeded in external functions. + error MessageSizeExceeded(); } /// @title IGatewayZEVM diff --git a/v2/test/GatewayEVM.t.sol b/v2/test/GatewayEVM.t.sol index 90aef05e..2bf0ef5d 100644 --- a/v2/test/GatewayEVM.t.sol +++ b/v2/test/GatewayEVM.t.sol @@ -456,6 +456,17 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR gateway.deposit{ value: amount }(address(0), revertOptions); } + function testDepositERC20ToCustodyWithPayloadFailsIfPayloadSizeExceeded() public { + uint256 amount = 100_000; + bytes memory payload = new bytes(512); + revertOptions.revertMessage = new bytes(512); + + token.approve(address(gateway), amount); + + vm.expectRevert(PayloadSizeExceeded.selector); + gateway.depositAndCall(destination, amount, address(token), payload, revertOptions); + } + function testDepositERC20ToCustodyWithPayloadFailsIfTokenIsNotWhitelisted() public { uint256 amount = 100_000; bytes memory payload = abi.encodeWithSignature("hello(address)", destination); @@ -520,6 +531,15 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR assertEq(tssBalanceBefore + amount, tssBalanceAfter); } + function testDepositEthToTssWithPayloadFailsIfPayloadSizeExceeded() public { + uint256 amount = 100_000; + bytes memory payload = new bytes(512); + revertOptions.revertMessage = new bytes(512); + + vm.expectRevert(PayloadSizeExceeded.selector); + gateway.depositAndCall{ value: amount }(destination, payload, revertOptions); + } + function testFailDepositEthToTssWithPayloadIfAmountIs0() public { uint256 amount = 0; bytes memory payload = abi.encodeWithSignature("hello(address)", destination); @@ -544,6 +564,14 @@ contract GatewayEVMInboundTest is Test, IGatewayEVMErrors, IGatewayEVMEvents, IR gateway.call(destination, payload, revertOptions); } + function testCallWithPayloadFailsIfPayloadSizeExceeded() public { + bytes memory payload = new bytes(512); + revertOptions.revertMessage = new bytes(512); + + vm.expectRevert(PayloadSizeExceeded.selector); + gateway.call(destination, payload, revertOptions); + } + function testCallWithPayloadFailsIfDestinationIsZeroAddress() public { bytes memory payload = abi.encodeWithSignature("hello(address)", destination); diff --git a/v2/test/GatewayZEVM.t.sol b/v2/test/GatewayZEVM.t.sol index d7a4a028..ea2ada45 100644 --- a/v2/test/GatewayZEVM.t.sol +++ b/v2/test/GatewayZEVM.t.sol @@ -149,6 +149,14 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors gateway.withdrawAndCall(abi.encodePacked(""), 1, address(zrc20), message, 1, revertOptions); } + function testWithdrawAndCallZRC20FailsIfMessageSizeExceeded() public { + bytes memory message = new bytes(512); + revertOptions.revertMessage = new bytes(512); + + vm.expectRevert(MessageSizeExceeded.selector); + gateway.withdrawAndCall(abi.encodePacked(addr1), 1, address(zrc20), message, 1, revertOptions); + } + function testWithdrawAndCallZRC20FailsIfGasLimitIsZero() public { bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectRevert(InsufficientGasLimit.selector); @@ -220,6 +228,13 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors gateway.withdrawAndCall(abi.encodePacked(addr1), 0, 1, message, revertOptions); } + function testWithdrawAndCallZETAFailsIfMessageSizeExceeded() public { + bytes memory message = new bytes(512); + revertOptions.revertMessage = new bytes(512); + vm.expectRevert(MessageSizeExceeded.selector); + gateway.withdrawAndCall(abi.encodePacked(addr1), 1, 1, message, revertOptions); + } + function testWithdrawAndCallZETAFailsIfAmountIsReceiverIsZeroAddress() public { bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectRevert(ZeroAddress.selector); @@ -336,6 +351,13 @@ contract GatewayZEVMInboundTest is Test, IGatewayZEVMEvents, IGatewayZEVMErrors gateway.call(abi.encodePacked(""), address(zrc20), message, 1, revertOptions); } + function testCallFailsIfMessageSizeExceeded() public { + bytes memory message = new bytes(512); + revertOptions.revertMessage = new bytes(512); + vm.expectRevert(MessageSizeExceeded.selector); + gateway.call(abi.encodePacked(addr1), address(zrc20), message, 1, revertOptions); + } + function testCallFailsIfGasLimitIsZero() public { bytes memory message = abi.encodeWithSignature("hello(address)", addr1); vm.expectRevert(InsufficientGasLimit.selector);