code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/SystemContext.sol#L116

Vulnerability details

Impact

Multiple blocks at the same timestamp creates ambiguity about the order in which these blocks should be added to the chain. This can cause inconsistencies in the state of the network and make it vulnerable to attacks such as double-spending.

Proof of Concept

require(_newTimestamp >= currentBlockTimestamp, "Timestamps should be incremental"); SystemContext.sol#L116

1) So as per _newTimestamp >= currentBlockTimestamp condition check its possible to add multiple blocks with same timestamp.

2) As per documentation the Timestamps should be incremental.

3) But the condition check is wrong. This will accept even the same timestamps because of >= condition check

Each block in a blockchain network contains a timestamp and a unique block hash, and the chronological order of the blocks is critical for the integrity and security of the network.

This can result in the creation of competing chains, known as forks, where different nodes on the network have different versions of the blockchain that they believe to be valid. This can cause inconsistencies in the state of the network and make it vulnerable to attacks such as double-spending

Tools Used

Manual Audit

Recommended Mitigation Steps

Modify the condition check like bellow

require(_newTimestamp > currentBlockTimestamp, "Timestamps should be incremental");

This ensures that _newTimestamp is greater than currentBlockTimestamp

c4-judge commented 1 year ago

GalloDaSballo marked the issue as duplicate of #59