Add reentrancy tests for standard gateway

OffchainLabs · Jan 26, 2024 · 55ccc23 · 55ccc23
1 parent 8170196
commit 55ccc23
Show file tree

Hide file tree

Showing 5 changed files with 95 additions and 64 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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;