v2.1.3 (#291)

* set decimals in postUpgradeInit * more intentional function name * comment * call it postUpgradeInit * add zero check * Apply 7702 patch * Updated slither * Linting * Storage formatting changes * Added post upgrade init signature * Added post upgrade init sig * chore: v2.1.3 --------- Co-authored-by: Henry <[email protected]> Co-authored-by: gzeon <[email protected]> Co-authored-by: gzeon <[email protected]>
OffchainLabs · Feb 5, 2025 · f9cd1aa · f9cd1aa
1 parent f843905
commit f9cd1aa
Show file tree

Hide file tree

Showing 11 changed files with 64 additions and 15 deletions.
diff --git a/audit-ci.jsonc b/audit-ci.jsonc
@@ -65,6 +65,8 @@
     // Server-Side Request Forgery in axios
     "GHSA-8hc4-vh64-cxmj",
     // Regular Expression Denial of Service (ReDoS) in micromatch
-    "GHSA-952p-6rrq-rcjv"
+    "GHSA-952p-6rrq-rcjv",
+    // cookie accepts cookie name, path, and domain with out of bounds characters
+    "GHSA-pxg6-pf52-xh8x"
   ]
 }
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@arbitrum/nitro-contracts",
-  "version": "2.1.2",
+  "version": "2.1.3",
   "description": "Layer 2 precompiles and rollup for Arbitrum Nitro",
   "author": "Offchain Labs, Inc.",
   "license": "BUSL-1.1",

diff --git a/slither.db.json b/slither.db.json
diff --git a/src/bridge/AbsInbox.sol b/src/bridge/AbsInbox.sol
@@ -11,14 +11,15 @@ import {
     InsufficientSubmissionCost,
     L1Forked,
     NotAllowedOrigin,
-    NotOrigin,
+    NotCodelessOrigin,
     NotRollupOrOwner,
     RetryableData
 } from "../libraries/Error.sol";
 import "./IInboxBase.sol";
 import "./ISequencerInbox.sol";
 import "./IBridge.sol";
 import "../libraries/AddressAliasHelper.sol";
+import "../libraries/CallerChecker.sol";
 import "../libraries/DelegateCallAware.sol";
 import {
     L1MessageType_submitRetryableTx,
@@ -138,8 +139,7 @@ abstract contract AbsInbox is DelegateCallAware, PausableUpgradeable, IInboxBase
         returns (uint256)
     {
         if (_chainIdChanged()) revert L1Forked();
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender != tx.origin) revert NotOrigin();
+        if (!CallerChecker.isCallerCodelessOrigin()) revert NotCodelessOrigin();
         if (messageData.length > maxDataSize) revert DataTooLarge(messageData.length, maxDataSize);
         uint256 msgNum = _deliverToBridge(L2_MSG, msg.sender, keccak256(messageData), 0);
         emit InboxMessageDeliveredFromOrigin(msgNum);

diff --git a/src/bridge/Inbox.sol b/src/bridge/Inbox.sol
@@ -118,6 +118,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();
@@ -152,6 +153,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();
@@ -185,6 +187,7 @@ contract Inbox is AbsInbox, IInbox {
         if (!_chainIdChanged()) revert NotForked();
         // solhint-disable-next-line avoid-tx-origin
         if (msg.sender != tx.origin) revert NotOrigin();
+        // no code size check required because we only want to know if msg.sender is an EOA to undo alias
         // arbos will discard unsigned tx with gas limit too large
         if (gasLimit > type(uint64).max) {
             revert GasLimitTooLarge();

diff --git a/src/bridge/SequencerInbox.sol b/src/bridge/SequencerInbox.sol
@@ -21,6 +21,7 @@ import {
     NoSuchKeyset,
     NotForked,
     NotBatchPosterManager,
+    NotCodelessOrigin,
     RollupNotChanged,
     DataBlobsNotSupported,
     InitParamZero,
@@ -38,6 +39,7 @@ import "../rollup/IRollupLogic.sol";
 import "./Messages.sol";
 import "../precompiles/ArbGasInfo.sol";
 import "../precompiles/ArbSys.sol";
+import "../libraries/CallerChecker.sol";
 import "../libraries/IReader4844.sol";
 
 import {L1MessageType_batchPostingReport} from "../libraries/MessageTypes.sol";
@@ -360,8 +362,7 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         uint256 prevMessageCount,
         uint256 newMessageCount
     ) external refundsGas(gasRefunder, IReader4844(address(0))) {
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender != tx.origin) revert NotOrigin();
+        if (!CallerChecker.isCallerCodelessOrigin()) revert NotCodelessOrigin();
         if (!isBatchPoster[msg.sender]) revert NotBatchPoster();
         (bytes32 dataHash, IBridge.TimeBounds memory timeBounds) = formCallDataHash(
             data,
@@ -457,10 +458,9 @@ contract SequencerInbox is DelegateCallAware, GasRefundEnabled, ISequencerInbox
         if (hostChainIsArbitrum) revert DataBlobsNotSupported();
 
         // submit a batch spending report to refund the entity that produced the blob batch data
-        // same as using calldata, we only submit spending report if the caller is the origin of the tx
+        // same as using calldata, we only submit spending report if the caller is the origin and is codeless
         // such that one cannot "double-claim" batch posting refund in the same tx
-        // solhint-disable-next-line avoid-tx-origin
-        if (msg.sender == tx.origin && !isUsingFeeToken) {
+        if (CallerChecker.isCallerCodelessOrigin() && !isUsingFeeToken) {
             submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, blobGas);
         }
     }

diff --git a/src/libraries/CallerChecker.sol b/src/libraries/CallerChecker.sol
@@ -0,0 +1,18 @@
+// Copyright 2021-2024, Offchain Labs, Inc.
+// For license information, see https://github.com/OffchainLabs/nitro-contracts/blob/main/LICENSE
+// SPDX-License-Identifier: BUSL-1.1
+
+pragma solidity ^0.8.0;
+
+library CallerChecker {
+    /**
+     * @notice A EIP-7702 safe check to ensure the caller is the origin and is codeless
+     * @return bool true if the caller is the origin and is codeless, false otherwise
+     * @dev    If the caller is the origin and is codeless, then msg.data is guaranteed to be same as tx.data
+     *         It also mean the caller would not be able to call a contract multiple times with the same transaction
+     */
+    function isCallerCodelessOrigin() internal view returns (bool) {
+        // solhint-disable-next-line avoid-tx-origin
+        return msg.sender == tx.origin && msg.sender.code.length == 0;
+    }
+}
diff --git a/src/libraries/Error.sol b/src/libraries/Error.sol
@@ -13,6 +13,9 @@ error HadZeroInit();
 /// @dev Thrown when post upgrade init validation fails
 error BadPostUpgradeInit();
 
+/// @dev Thrown when the caller is not a codeless origin
+error NotCodelessOrigin();
+
 /// @dev Thrown when non owner tries to access an only-owner function
 /// @param sender The msg.sender who is not the owner
 /// @param owner The owner address

diff --git a/src/libraries/GasRefundEnabled.sol b/src/libraries/GasRefundEnabled.sol
@@ -7,6 +7,7 @@ pragma solidity ^0.8.0;
 
 import "./IReader4844.sol";
 import "./IGasRefunder.sol";
+import "../libraries/CallerChecker.sol";
 
 abstract contract GasRefundEnabled {
     uint256 internal immutable gasPerBlob = 2**17;
@@ -24,8 +25,7 @@ abstract contract GasRefundEnabled {
             startGasLeft += calldataWords * 6 + (calldataWords**2) / 512;
             // if triggered in a contract call, the spender may be overrefunded by appending dummy data to the call
             // so we check if it is a top level call, which would mean the sender paid calldata as part of tx.input
-            // solhint-disable-next-line avoid-tx-origin
-            if (msg.sender != tx.origin) {
+            if (!CallerChecker.isCallerCodelessOrigin()) {
                 // We can't be sure if this calldata came from the top level tx,
                 // so to be safe we tell the gas refunder there was no calldata.
                 calldataSize = 0;

diff --git a/test/foundry/AbsInbox.t.sol b/test/foundry/AbsInbox.t.sol
@@ -230,10 +230,19 @@ abstract contract AbsInboxTest is Test {
     }
 
     function test_sendL2MessageFromOrigin_revert_NotOrigin() public {
-        vm.expectRevert(abi.encodeWithSelector(NotOrigin.selector));
+        vm.expectRevert(abi.encodeWithSelector(NotCodelessOrigin.selector));
         inbox.sendL2MessageFromOrigin(abi.encodePacked("some msg"));
     }
 
+    function test_sendL2MessageFromOrigin_revert_NotCodeless() public {
+        assertEq(user.code.length, 0, "user is codeless");
+        vm.etch(user, bytes("some code"));
+        vm.prank(user, user);
+        vm.expectRevert(abi.encodeWithSelector(NotCodelessOrigin.selector));
+        inbox.sendL2MessageFromOrigin(abi.encodePacked("some msg"));
+        vm.etch(user, bytes(""));
+    }
+
     function test_sendL2Message() public {
         // L2 msg params
         bytes memory data = abi.encodePacked("some msg");

diff --git a/test/foundry/SequencerInbox.t.sol b/test/foundry/SequencerInbox.t.sol
@@ -409,7 +409,7 @@ contract SequencerInboxTest is Test {
         uint256 sequenceNumber = bridge.sequencerMessageCount();
         uint256 delayedMessagesRead = bridge.delayedMessageCount();
 
-        vm.expectRevert(abi.encodeWithSelector(NotOrigin.selector));
+        vm.expectRevert(abi.encodeWithSelector(NotCodelessOrigin.selector));
         seqInbox.addSequencerL2BatchFromOrigin(
             sequenceNumber,
             data,
@@ -419,6 +419,20 @@ contract SequencerInboxTest is Test {
             subMessageCount + 1
         );
 
+        assertEq(rollupOwner.code.length, 0, "rollupOwner is codeless");
+        vm.etch(rollupOwner, bytes("some code"));
+        vm.prank(rollupOwner, rollupOwner);
+        vm.expectRevert(abi.encodeWithSelector(NotCodelessOrigin.selector));
+        seqInbox.addSequencerL2BatchFromOrigin(
+            sequenceNumber,
+            data,
+            delayedMessagesRead,
+            IGasRefunder(address(0)),
+            subMessageCount,
+            subMessageCount + 1
+        );
+        vm.etch(rollupOwner, bytes(""));
+
         vm.prank(rollupOwner);
         seqInbox.setIsBatchPoster(tx.origin, false);