Wrong usage of origin checking in sequence inbox

[c4-bot-8]

Lines of code

https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/bridge/SequencerInbox.sol#L577 https://github.com/code-423n4/2024-05-arbitrum-foundation/blob/6f861c85b281a29f04daacfe17a2099d7dad5f8f/src/bridge/SequencerInbox.sol#L601

Vulnerability details

Impact

Batch posters will not get their gas fees reimbursed.

Proof of Concept

Using Arbitrum's SequencerInbox contract, sequencers(or aka batch posters) submit tx batches through some methods like addSequencerL2Batch or addSequencerL2BatchDelayProof. The sequencers get their gas fee reimbursed based on the length of the data they post, as the following code snippet shows. SequencerInbox.sol#L818-L821

if (calldataLengthPosted > 0 && !isUsingFeeToken) {
    // only report batch poster spendings if chain is using ETH as native currency
    submitBatchSpendingReport(dataHash, seqMessageIndex, block.basefee, 0);
}

However, in addSequencerL2Batch and addSequencerL2BatchDelayProof functions, they pass false as isFromOrigin flag, regardless of msg.sender. In fact, these functions are called by either whitelisted EOA batch posters or the rollup contract, which means when these functions are called by whitelisted EOA batch posters, they will not be able to get their gas fees reimbursed.

Tools Used

Manual Review

Recommended Mitigation Steps

It should check if msg.sender is an EOA and pass true if that's the case.

addSequencerL2BatchFromCalldataImpl(
    sequenceNumber,
    data,
    afterDelayedMessagesRead,
    prevMessageCount,
    newMessageCount,
-   false
+   msg.sender == tx.origin
);

Assessed type

Context

[c4-sponsor]

godzillaba (sponsor) disputed

[gzeoneth]

Out-of-scope (code existed before BOLD). Also invalid as the batch poster should use FromOrigin functions when calling directly from an EOA.

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid