Revert batch does not clear the state properly in executor facet

[c4-submissions]

Lines of code

ttps://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L212 https://github.com/code-423n4/2023-10-zksync/blob/1fb4649b612fac7b4ee613df6f6b7d921ddd6b0d/code/contracts/ethereum/contracts/zksync/facets/Executor.sol#L206

Vulnerability details

Impact

Revert batch does not clear the state properly in executor facet

Proof of Concept

In the current flow of batch transaction on executor facet

the validator

1. commitBatches
2. proveBatches
3. executeBatches

the executed batches cannot be revert

but if the batch is not executed, even the batch is commited and proved

the validator can revert the batch

but when revert the batch, the state that are updated during commitBatch and proveBatches and not properly cleared

during commit batch, we already update the state s.storedBatchHashes update s.storedBatchHashes in both commit that trigger system upgrade and commit that does not trigger system upgrade

for (uint256 i = 0; i < _newBatchesData.length; i = i.uncheckedInc()) {
    _lastCommittedBatchData = _commitOneBatch(_lastCommittedBatchData, _newBatchesData[i], bytes32(0));

    s.storedBatchHashes[_lastCommittedBatchData.batchNumber] = _hashStoredBatchInfo(_lastCommittedBatchData);

but when reverting the batch, the outdated s.storedBatchHashes is never cleared and removed

then later when commiting other batch, the validator has to supply the wrong and oudated lastBatchCommitData

Tools Used

Manual Review

Recommended Mitigation Steps

clear and reset the state of s.storedBatchHashes

Assessed type

Other

[bytes032]

Could be similar to #114

[c4-pre-sort]

bytes032 marked the issue as sufficient quality report

[c4-pre-sort]

141345 marked the issue as duplicate of #117

[miladpiri]

Not needed. The total number of batches committed is a critical number. Not duplicate of 117.

[c4-sponsor]

miladpiri (sponsor) disputed

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)

[c4-judge]

GalloDaSballo marked the issue as not a duplicate

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid