Incorrect Check for Intrinsic Costs in L1 to L2 Transaction Validation

[c4-bot-3]

Lines of code

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/libraries/TransactionValidator.sol#L35-L41

Vulnerability details

Impact

The current implementation of the validateL1ToL2Transaction function allows an L1 to L2 transaction to be sent that does not cover the sum of overhead and intrinsic costs for the operator. This could potentially enable griefing attacks by submitting transactions that only cover the maximum of the overhead or intrinsic costs, but not the total sum of both.

Proof of Concept

https://github.com/code-423n4/2024-03-zksync/blob/4f0ba34f34a864c354c7e8c47643ed8f4a250e13/code/contracts/ethereum/contracts/state-transition/libraries/TransactionValidator.sol#L35-L41

The validateL1ToL2Transaction function checks the minimal priority transaction gas limit calculated by getMinimalPriorityTransactionGasLimit against the _transaction.gasLimit. However, _transaction.gasLimit represents the total gas limit, including the overhead. The correct check should be against the actual gas limit available for execution, which is calculated by subtracting the overhead from the total gas limit.

Tools Used

God & Manual Review

Recommended Mitigation Steps

1. In the validateL1ToL2Transaction function, I'd suggest replacing the check:

require(
    getMinimalPriorityTransactionGasLimit(
        _encoded.length,
        _transaction.factoryDeps.length,
        _transaction.gasPerPubdataByteLimit
    ) <= _transaction.gasLimit,
    "up"
);

with:

uint256 txBodyGasLimit = getTransactionBodyGasLimit(_transaction.gasLimit, _encoded.length);
require(
    getMinimalPriorityTransactionGasLimit(
        _encoded.length,
        _transaction.factoryDeps.length,
        _transaction.gasPerPubdataByteLimit
    ) <= txBodyGasLimit,
    "up"
);

2. This way, the minimal priority transaction gas limit (which represents the sum of intrinsic costs and execution costs) is correctly checked against the actual gas limit available for execution (txBodyGasLimit), ensuring that the transaction covers both the overhead and the intrinsic costs.

Assessed type

Other

[c4-sponsor]

saxenism (sponsor) disputed

[saxenism]

This is an invalid issue first of all because this is out of scope and secondly since the design was made with this possibility already in mind

[alex-ppg]

The Warden specifies that the minimal priority transaction gas limit is incorrectly assessed due to factoring in the gas overhead supplied by the operator.

The Sponsor states that the issue is "out-of-scope", however, the contract concerned was within the scope of the contest. The described submission is out-of-scope, however, as a "known-issue" due to the NOTE in-line documentation that accompanies the function as referenced by the Sponsor here, effectively classifying the discrepancy outlined by the Warden as known behavior.

As a final note, I would like to clarify that there is no place for any form of religious reference, even in the abstract reference of "God", due to the presence of atheism as well as polytheistic religions. Such references can directly influence the perception of a finding from the Sponsor, Judge, Wardens, as well as future readers of the audit report, all of which are undesirable and have no place in an impartial audit report.

[c4-judge]

alex-ppg marked the issue as unsatisfactory: Out of scope