search

code-423n4 / 2023-03-zksync-findings

5 stars 0 forks source link

time-sensitive contracts deployed on zkSync #70

Open code423n4 opened 1 year ago

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

Time-sensitive contracts will be impacted if deployed on zkSync.

Proof of Concept

Many contracts use block.number to measure the time as the miners were able to manipulate the timestamp (the timestamp could be easily gamed over short intervals). So, it was assumed that block.number is a safer and more accurate source of measuring time than timestamp.

For instance, if a defi project sets 144000 block interval to release the interest, it means approximately 144000 * 12 = 20 days. Please note that each block in Ethereum takes almost 12 second.

If the same defi project is deployed on zkSync, it will not operate as expected. Because the there is no time-bound for the blocks in zkSync (the interval may be 30 seconds or 1 week). So, the time to release the interest can be between 50 days to 2762 days.

Since, it is assumed that zkSync is Ethereum compatible, any deployed contracts on Ethereum may deploy their contract in zkSync without noting such big difference.

Even if the contracts use timestamp to measure the time, there will be another issue. In the contract SystemContext.sol, it is possible to set new block with the same timestamp as previous block, but with incremented block number. https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/SystemContext.sol#L116

In other words, new blocks are created but their time is frozen. Please note that freezing time can not be lasted for a long time, because when committing block their timestamp will be validated against a defined boundary.

Tools Used

Recommended Mitigation Steps

It should be explicitly mentioned that block intervals in zkSync are not compatible with Ethereum. So, time-sensitive contracts will be noted.

Moreover, the equal sign should be removed in the following line: https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/contracts/SystemContext.sol#L116

c4-judge commented 1 year ago

GalloDaSballo marked the issue as primary issue

miladpiri commented 1 year ago

Two issues are stated:

  1. block timestamp issue (which is duplicate of some other reports). dup #31 (Low)
  2. Block creation rate is not consistent with Ethereum. (not duplicate) (Medium)

The judges can decide better how to distinguish them.

c4-sponsor commented 1 year ago

miladpiri marked the issue as sponsor confirmed

GalloDaSballo commented 1 year ago

The warden has shown how the zkEVM could differ from the EVM in how block timing is enforced, because blocks can happen at inconsistent times, protocols contract could have their time assumptions broken.

Because this is a finding that has been addressed by other L2s, and causes inconsistent behaviour vs the EVM, I agree with Medium Severity

c4-judge commented 1 year ago

GalloDaSballo marked the issue as selected for report

Minh-Trng commented 1 year ago

Because this is a finding that has been addressed by other L2s, and causes inconsistent behaviour vs the EVM, I agree with Medium Severity

which L2s are those and how do they fix it? Intuitively I would have thought, that the discrepancy of blocktime between Ethereum and L2s is common sense and innate, because the whole thing of an L2 is to process multiple blocks and compress them before posting them into a single L1 block.

Have checked it only for arbitrum and optimism, but they have the same "issue": https://developer.arbitrum.io/time#block-numbers-arbitrum-vs-ethereum https://community.optimism.io/docs/developers/build/differences/#block-production-is-not-constant

GalloDaSballo commented 1 year ago

Because this is a finding that has been addressed by other L2s, and causes inconsistent behaviour vs the EVM, I agree with Medium Severity

which L2s are those and how do they fix it? Intuitively I would have thought, that the discrepancy of blocktime between Ethereum and L2s is common sense and innate, because the whole thing of an L2 is to process multiple blocks and compress them before posting them into a single L1 block.

Have checked it only for arbitrum and optimism, but they have the same "issue": https://developer.arbitrum.io/time#block-numbers-arbitrum-vs-ethereum https://community.optimism.io/docs/developers/build/differences/#block-production-is-not-constant

You can see how Arbitrum does it in the link you sent and how protocols must adapt in order to use block values, for example, see GMX's fix: https://github.com/gmx-io/gmx-synthetics/blob/main/contracts/chain/Chain.sol

For OP you can see that they are working on a fix to offer consistent times: https://community.optimism.io/docs/developers/bedrock/differences/#the-evm