Fix Macro L2 audit finding (#15)

* Add checkValue modifier to L1XERC20Adapter and ArbitrumEnabledXER20 * Add L1 fix tests * Lint * Update test/L1ArbitrumEnabledXERC20.t.sol Co-authored-by: Tomas Rojas <tmsrjs@gmail.com> --------- Co-authored-by: Tomas Rojas <tmsrjs@gmail.com>
BootNodeDev · Jun 27, 2024 · fbb173e · fbb173e
1 parent 6d9f114
commit fbb173e
Show file tree

Hide file tree

Showing 9 changed files with 161 additions and 6 deletions.
diff --git a/bun.lockb b/bun.lockb
diff --git a/src/L1ArbitrumEnabledXERC20.sol b/src/L1ArbitrumEnabledXERC20.sol
@@ -53,6 +53,7 @@ contract L1ArbitrumEnabledXERC20 is XERC20, L1ArbitrumEnabled {
         payable
         override
         onlyOwner
+        checkValue(valueForGateway + valueForRouter)
     {
         _registerTokenOnL2(
             l2TokenAddress,

diff --git a/src/L1XERC20Adapter.sol b/src/L1XERC20Adapter.sol
@@ -55,6 +55,7 @@ contract L1XERC20Adapter is XERC20BaseAdapter, L1ArbitrumEnabled {
         payable
         override
         onlyOwner
+        checkValue(valueForGateway + valueForRouter)
     {
         _registerTokenOnL2(
             l2TokenAddress,

diff --git a/src/libraries/L1ArbitrumEnabled.sol b/src/libraries/L1ArbitrumEnabled.sol
@@ -15,13 +15,21 @@ import { IL1GatewayRouter } from "src/interfaces/IL1GatewayRouter.sol";
 abstract contract L1ArbitrumEnabled {
     address internal gatewayAddress;
 
+    error WrongValue();
+
     /**
      * @dev Sets the address of the gateway to be used for bridging
      */
     constructor(address _gatewayAddress) {
         gatewayAddress = _gatewayAddress;
     }
 
+    modifier checkValue(uint256 _value) {
+        if (_value != msg.value) revert WrongValue();
+
+        _;
+    }
+
     /**
      * @dev Returns a magic value expected by the Arbitrum Router.
      */

diff --git a/test/L1ArbitrumEnabledXERC20.t.sol b/test/L1ArbitrumEnabledXERC20.t.sol
@@ -0,0 +1,67 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.25 <0.9.0;
+
+import { Test } from "forge-std/Test.sol";
+
+// solhint-disable-next-line
+import { console2 } from "forge-std/console2.sol";
+
+import { L1ArbitrumEnabledXERC20 } from "src/L1ArbitrumEnabledXERC20.sol";
+import { L1ArbitrumEnabled } from "src/libraries/L1ArbitrumEnabled.sol";
+
+import { GatewayMock } from "test/mocks/GatewayMock.sol";
+import { RouterMock } from "test/mocks/RouterMock.sol";
+
+contract L1ArbitrumEnabledXERC20Test is Test {
+    GatewayMock internal gateway;
+    L1ArbitrumEnabledXERC20 internal arbEnabledToken;
+    address internal _owner = makeAddr("owner");
+
+    function setUp() public {
+        gateway = new GatewayMock();
+
+        arbEnabledToken = new L1ArbitrumEnabledXERC20("ArbitrumEnabledToken", "AET", _owner, address(gateway));
+    }
+
+    function test_IsArbitrumEnabled() public view {
+        assertEq(arbEnabledToken.isArbitrumEnabled(), uint8(0xb1));
+    }
+
+    function test_registerTokenOnL2_WrongValue(uint256 valueForGateway, uint256 valueForRouter) public {
+        // bound to avoid overflow or underflow
+        valueForGateway = bound(valueForGateway, 1, 1e36);
+        valueForRouter = bound(valueForRouter, 1, 1e36);
+
+        deal(_owner, valueForGateway + valueForRouter + 1);
+
+        vm.prank(_owner);
+        vm.expectRevert(L1ArbitrumEnabled.WrongValue.selector);
+        arbEnabledToken.registerTokenOnL2{ value: valueForGateway + valueForRouter - 1 }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+
+        vm.prank(_owner);
+        vm.expectRevert(L1ArbitrumEnabled.WrongValue.selector);
+        arbEnabledToken.registerTokenOnL2{ value: valueForGateway + valueForRouter + 1 }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+    }
+
+    function test_registerTokenOnL2_works(uint256 valueForGateway, uint256 valueForRouter) public {
+        // bound to avoid overflow or underflow
+        valueForGateway = bound(valueForGateway, 1, 1e36);
+        valueForRouter = bound(valueForRouter, 1, 1e36);
+
+        deal(_owner, valueForGateway + valueForRouter);
+
+        address router = gateway.router();
+
+        vm.prank(_owner);
+        vm.expectCall(address(gateway), valueForGateway, abi.encodePacked(GatewayMock.registerTokenToL2.selector));
+        vm.expectCall(address(gateway), abi.encodePacked(GatewayMock.router.selector));
+        vm.expectCall(router, valueForRouter, abi.encodePacked(RouterMock.setGateway.selector));
+        arbEnabledToken.registerTokenOnL2{ value: valueForGateway + valueForRouter }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+    }
+}
diff --git a/test/L1XERC20AdapterTest.t.sol b/test/L1XERC20AdapterTest.t.sol
@@ -5,15 +5,63 @@ pragma solidity >=0.8.25 <0.9.0;
 import { console2 } from "forge-std/console2.sol";
 
 import { L1XERC20Adapter } from "src/L1XERC20Adapter.sol";
+import { L1ArbitrumEnabled } from "src/libraries/L1ArbitrumEnabled.sol";
 
 import { XERC20BaseAdapterTest } from "test/XERC20BaseAdapterTest.t.sol";
+import { GatewayMock } from "test/mocks/GatewayMock.sol";
+import { RouterMock } from "test/mocks/RouterMock.sol";
 
 contract L1XERC20AdapterTest is XERC20BaseAdapterTest {
+    GatewayMock internal gateway;
+
+    function setUp() public override {
+        gateway = new GatewayMock();
+        super.setUp();
+    }
+
     function test_IsArbitrumEnabled() public view {
         assertEq(L1XERC20Adapter(_adapter).isArbitrumEnabled(), uint8(0xb1));
     }
 
+    function test_registerTokenOnL2_WrongValue(uint256 valueForGateway, uint256 valueForRouter) public {
+        // bound to avoid overflow or underflow
+        valueForGateway = bound(valueForGateway, 1, 1e36);
+        valueForRouter = bound(valueForRouter, 1, 1e36);
+
+        deal(_owner, valueForGateway + valueForRouter + 1);
+
+        vm.prank(_owner);
+        vm.expectRevert(L1ArbitrumEnabled.WrongValue.selector);
+        L1XERC20Adapter(_adapter).registerTokenOnL2{ value: valueForGateway + valueForRouter - 1 }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+
+        vm.prank(_owner);
+        vm.expectRevert(L1ArbitrumEnabled.WrongValue.selector);
+        L1XERC20Adapter(_adapter).registerTokenOnL2{ value: valueForGateway + valueForRouter + 1 }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+    }
+
+    function test_registerTokenOnL2_works(uint256 valueForGateway, uint256 valueForRouter) public {
+        // bound to avoid overflow or underflow
+        valueForGateway = bound(valueForGateway, 1, 1e36);
+        valueForRouter = bound(valueForRouter, 1, 1e36);
+
+        deal(_owner, valueForGateway + valueForRouter);
+
+        address router = gateway.router();
+
+        vm.prank(_owner);
+        vm.expectCall(address(gateway), valueForGateway, abi.encodePacked(GatewayMock.registerTokenToL2.selector));
+        vm.expectCall(address(gateway), abi.encodePacked(GatewayMock.router.selector));
+        vm.expectCall(router, valueForRouter, abi.encodePacked(RouterMock.setGateway.selector));
+        L1XERC20Adapter(_adapter).registerTokenOnL2{ value: valueForGateway + valueForRouter }(
+            makeAddr("l2Token"), 0, 0, 0, 0, 0, valueForGateway, valueForRouter, makeAddr("creditBack")
+        );
+    }
+
     function _createAdapter() internal override {
-        _adapter = address(new L1XERC20Adapter(address(xerc20), makeAddr("gateway"), _owner));
+        _adapter = address(new L1XERC20Adapter(address(xerc20), address(gateway), _owner));
     }
 }
diff --git a/test/forking/L1XERC20Gateway.t.sol b/test/forking/L1XERC20Gateway.t.sol
@@ -40,7 +40,7 @@ contract L1XERC20GatewayForkingTest is L1XERC20BaseGatewayTest {
         l1GatewayRouter = 0x72Ce9c846789fdB6fC1f34aC4AD25Dd9ef7031ef;
         l1Inbox = 0x4Dbd4fc535Ac27206064B68FfCf827b0A60BAB3f;
 
-        deal(_owner, 100 ether);
+        deal(_owner, 100 ether + retryableCost * 2);
         deal(_attacker, 100 ether);
 
         super.setUp();
@@ -49,7 +49,7 @@ contract L1XERC20GatewayForkingTest is L1XERC20BaseGatewayTest {
 
     function test_RegisterTokenOnL2() public {
         vm.prank(_owner);
-        ICustomToken(bridgeable).registerTokenOnL2{ value: 3 ether }(
+        ICustomToken(bridgeable).registerTokenOnL2{ value: retryableCost * 2 }(
             l2TokenAddress,
             maxSubmissionCost,
             maxSubmissionCost,
@@ -74,7 +74,7 @@ contract L1XERC20GatewayForkingTest is L1XERC20BaseGatewayTest {
 
         vm.expectRevert(L1XERC20Gateway.AlreadyRegisteredL1Token.selector);
         vm.prank(_attacker);
-        ICustomToken(address(fakeAdapter)).registerTokenOnL2{ value: 3 ether }(
+        ICustomToken(address(fakeAdapter)).registerTokenOnL2{ value: retryableCost * 2 }(
             makeAddr("fakeL2TokenAddress"),
             maxSubmissionCost,
             maxSubmissionCost,
@@ -98,7 +98,7 @@ contract L1XERC20GatewayForkingTest is L1XERC20BaseGatewayTest {
 
         vm.expectRevert(L1XERC20Gateway.AlreadyRegisteredL2Token.selector);
         vm.prank(_attacker);
-        ICustomToken(address(fakeAdapter)).registerTokenOnL2{ value: 3 ether }(
+        ICustomToken(address(fakeAdapter)).registerTokenOnL2{ value: retryableCost * 2 }(
             l2TokenAddress,
             maxSubmissionCost,
             maxSubmissionCost,
@@ -113,7 +113,7 @@ contract L1XERC20GatewayForkingTest is L1XERC20BaseGatewayTest {
 
     function test_OutboundTransferCustomRefund() public {
         vm.prank(_owner);
-        ICustomToken(bridgeable).registerTokenOnL2{ value: 3 ether }(
+        ICustomToken(bridgeable).registerTokenOnL2{ value: retryableCost * 2 }(
             l2TokenAddress,
             maxSubmissionCost,
             maxSubmissionCost,

diff --git a/test/mocks/GatewayMock.sol b/test/mocks/GatewayMock.sol
@@ -0,0 +1,20 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.25 <0.9.0;
+
+import { RouterMock } from "./RouterMock.sol";
+
+contract GatewayMock {
+    RouterMock internal routerContract;
+
+    constructor() {
+        routerContract = new RouterMock();
+    }
+
+    function registerTokenToL2(address, uint256, uint256, uint256, address) public payable returns (uint256) {
+        return 1;
+    }
+
+    function router() public view returns (address) {
+        return address(routerContract);
+    }
+}
diff --git a/test/mocks/RouterMock.sol b/test/mocks/RouterMock.sol
@@ -0,0 +1,10 @@
+// SPDX-License-Identifier: MIT
+pragma solidity >=0.8.25 <0.9.0;
+
+import { IL1CustomGateway } from "src/interfaces/IL1CustomGateway.sol";
+
+contract RouterMock {
+    function setGateway(IL1CustomGateway, uint256, uint256, uint256, address) public payable returns (uint256) {
+        return 1;
+    }
+}