diff --git a/test-foundry/L1ArbitrumGateway.t.sol b/test-foundry/L1ArbitrumGateway.t.sol index 4f2c8235a..7e29c35a5 100644 --- a/test-foundry/L1ArbitrumGateway.t.sol +++ b/test-foundry/L1ArbitrumGateway.t.sol @@ -190,3 +190,38 @@ contract L1ArbitrumGatewayMock is L1ArbitrumGateway { return x; } } + +contract MockReentrantInbox { + function createRetryableTicket( + address, + uint256, + uint256, + address, + address, + uint256, + uint256, + bytes memory + ) external payable returns (uint256) { + // re-enter + L1ArbitrumGateway(msg.sender).outboundTransferCustomRefund{value: msg.value}( + address(100), address(100), address(100), 2, 2, 2, bytes("") + ); + } +} + +contract MockReentrantERC20 { + function balanceOf(address) external returns (uint256) { + // re-enter + L1ArbitrumGateway(msg.sender).outboundTransferCustomRefund( + address(100), address(100), address(100), 2, 2, 3, bytes("") + ); + return 5; + } + + function bridgeBurn(address, uint256) external { + // re-enter + L1ArbitrumGateway(msg.sender).outboundTransferCustomRefund( + address(100), address(100), address(100), 2, 2, 3, bytes("") + ); + } +} diff --git a/test-foundry/L1CustomGateway.t.sol b/test-foundry/L1CustomGateway.t.sol index 91094a274..34b69aa1d 100644 --- a/test-foundry/L1CustomGateway.t.sol +++ b/test-foundry/L1CustomGateway.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import {L1ArbitrumExtendedGatewayTest} from "./L1ArbitrumExtendedGateway.t.sol"; +import "./L1ArbitrumExtendedGateway.t.sol"; import { L1CustomGateway, IInbox, @@ -592,21 +592,3 @@ contract L1CustomGatewayTest is L1ArbitrumExtendedGatewayTest { event RefundAddresses(address excessFeeRefundAddress, address callValueRefundAddress); event InboxRetryableTicket(address from, address to, uint256 value, uint256 maxGas, bytes data); } - -contract MockReentrantInbox { - function createRetryableTicket( - address, - uint256, - uint256, - address, - address, - uint256, - uint256, - bytes memory - ) external payable returns (uint256) { - // re-enter - L1CustomGateway(msg.sender).outboundTransferCustomRefund{value: msg.value}( - address(100), address(100), address(100), 2, 2, 2, bytes("") - ); - } -} diff --git a/test-foundry/L1ERC20Gateway.t.sol b/test-foundry/L1ERC20Gateway.t.sol index 6602f8397..66bc820c9 100644 --- a/test-foundry/L1ERC20Gateway.t.sol +++ b/test-foundry/L1ERC20Gateway.t.sol @@ -2,9 +2,7 @@ pragma solidity ^0.8.0; -import { - L1ArbitrumExtendedGatewayTest, InboxMock, TestERC20 -} from "./L1ArbitrumExtendedGateway.t.sol"; +import "./L1ArbitrumExtendedGateway.t.sol"; import "contracts/tokenbridge/ethereum/gateway/L1ERC20Gateway.sol"; import "@openzeppelin/contracts/token/ERC20/ERC20.sol"; @@ -200,6 +198,29 @@ contract L1ERC20GatewayTest is L1ArbitrumExtendedGatewayTest { ); } + function test_outboundTransferCustomRefund_revert_Reentrancy() public virtual { + // approve token + uint256 depositAmount = 3; + vm.prank(user); + token.approve(address(l1Gateway), depositAmount); + + // trigger re-entrancy + MockReentrantInbox mockReentrantInbox = new MockReentrantInbox(); + vm.etch(l1Gateway.inbox(), address(mockReentrantInbox).code); + + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(router); + l1Gateway.outboundTransferCustomRefund{value: retryableCost}( + address(token), + makeAddr("refundTo"), + user, + depositAmount, + maxGas, + gasPriceBid, + buildRouterEncodedData("") + ); + } + function test_getOutboundCalldata() public { bytes memory outboundCalldata = l1Gateway.getOutboundCalldata({ _token: address(token), diff --git a/test-foundry/L1OrbitCustomGateway.t.sol b/test-foundry/L1OrbitCustomGateway.t.sol index a358c39e5..2c9a31b73 100644 --- a/test-foundry/L1OrbitCustomGateway.t.sol +++ b/test-foundry/L1OrbitCustomGateway.t.sol @@ -2,7 +2,7 @@ pragma solidity ^0.8.0; -import {L1CustomGatewayTest, IERC20, L2CustomGateway} from "./L1CustomGateway.t.sol"; +import "./L1CustomGateway.t.sol"; import {L1OrbitCustomGateway} from "contracts/tokenbridge/ethereum/gateway/L1OrbitCustomGateway.sol"; import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; import {ERC20PresetMinterPauser} from @@ -642,20 +642,3 @@ contract L1OrbitCustomGatewayTest is L1CustomGatewayTest { bytes data ); } - -contract MockReentrantERC20 { - function balanceOf(address) external returns (uint256) { - // re-enter - L1OrbitCustomGateway(msg.sender).outboundTransferCustomRefund( - address(100), address(100), address(100), 2, 2, 3, bytes("") - ); - return 5; - } - - function bridgeBurn(address, uint256) external { - // re-enter - L1OrbitCustomGateway(msg.sender).outboundTransferCustomRefund( - address(100), address(100), address(100), 2, 2, 3, bytes("") - ); - } -} diff --git a/test-foundry/L1OrbitERC20Gateway.t.sol b/test-foundry/L1OrbitERC20Gateway.t.sol index ecd7a7829..f7a95ee46 100644 --- a/test-foundry/L1OrbitERC20Gateway.t.sol +++ b/test-foundry/L1OrbitERC20Gateway.t.sol @@ -2,12 +2,13 @@ pragma solidity ^0.8.0; -import { L1ERC20GatewayTest } from "./L1ERC20Gateway.t.sol"; +import "./L1ERC20Gateway.t.sol"; import "contracts/tokenbridge/ethereum/gateway/L1OrbitERC20Gateway.sol"; -import { ERC20 } from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; -import { ERC20PresetMinterPauser } from "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; -import { TestERC20 } from "contracts/tokenbridge/test/TestERC20.sol"; -import { ERC20InboxMock } from "contracts/tokenbridge/test/InboxMock.sol"; +import {ERC20} from "@openzeppelin/contracts/token/ERC20/ERC20.sol"; +import {ERC20PresetMinterPauser} from + "@openzeppelin/contracts/token/ERC20/presets/ERC20PresetMinterPauser.sol"; +import {TestERC20} from "contracts/tokenbridge/test/TestERC20.sol"; +import {ERC20InboxMock} from "contracts/tokenbridge/test/InboxMock.sol"; contract L1OrbitERC20GatewayTest is L1ERC20GatewayTest { ERC20 public nativeToken; @@ -21,11 +22,7 @@ contract L1OrbitERC20GatewayTest is L1ERC20GatewayTest { l1Gateway = new L1OrbitERC20Gateway(); L1OrbitERC20Gateway(address(l1Gateway)).initialize( - l2Gateway, - router, - inbox, - cloneableProxyHash, - l2BeaconProxyFactory + l2Gateway, router, inbox, cloneableProxyHash, l2BeaconProxyFactory ); token = IERC20(address(new TestERC20())); @@ -92,12 +89,7 @@ contract L1OrbitERC20GatewayTest is L1ERC20GatewayTest { // trigger deposit vm.prank(router); l1Gateway.outboundTransfer( - address(token), - user, - depositAmount, - maxGas, - gasPriceBid, - routerEncodedData + address(token), user, depositAmount, maxGas, gasPriceBid, routerEncodedData ); // check tokens are escrowed @@ -187,13 +179,34 @@ contract L1OrbitERC20GatewayTest is L1ERC20GatewayTest { vm.prank(router); vm.expectRevert("NOT_ALLOWED_TO_BRIDGE_FEE_TOKEN"); l1Gateway.outboundTransferCustomRefund( - address(nativeToken), + address(nativeToken), creditBackAddress, user, 100, maxGas, gasPriceBid, "" + ); + } + + function test_outboundTransferCustomRefund_revert_Reentrancy() public override { + // approve fees + vm.prank(user); + nativeToken.approve(address(l1Gateway), nativeTokenTotalFee); + + // approve token + uint256 depositAmount = 3; + vm.prank(user); + token.approve(address(l1Gateway), depositAmount); + + // trigger re-entrancy + MockReentrantERC20 mockReentrantERC20 = new MockReentrantERC20(); + vm.etch(address(token), address(mockReentrantERC20).code); + + vm.expectRevert("ReentrancyGuard: reentrant call"); + vm.prank(router); + l1Gateway.outboundTransferCustomRefund( + address(token), creditBackAddress, user, - 100, + depositAmount, maxGas, gasPriceBid, - "" + buildRouterEncodedData("") ); } @@ -206,11 +219,8 @@ contract L1OrbitERC20GatewayTest is L1ERC20GatewayTest { override returns (bytes memory) { - bytes memory userEncodedData = abi.encode( - maxSubmissionCost, - callHookData, - nativeTokenTotalFee - ); + bytes memory userEncodedData = + abi.encode(maxSubmissionCost, callHookData, nativeTokenTotalFee); bytes memory routerEncodedData = abi.encode(user, userEncodedData); return routerEncodedData;