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

5 stars 1 forks source link

[bootloader] A bytecode hash without the bytecode (preimage) can be marked as known, breaking the prover #143

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/21d9a364a4a75adfa6f1e038232d8c0f39858a64/bootloader/bootloader.yul#L1324-L1345

Vulnerability details

This is a report of a finding in bootloader.yul. While the file is out of scope of the contest, the sponsor stated that they would still accept findings in the file and would judge them separately from the contest.

Impact

A bytecode hash for which the bytecode (preimage) wasn't published to the L1 can be marked as known on the L2. This will break mining and proving of blocks because calls to contracts with invalid or missing bytecode cannot be proved. The issue cannot be resolved after deployment: the preimage (i.e. bytecode) of a butecode hash that has been marked as known cannot be re-published to the L1 since the bootloader doesn't allow publishing known bytecodes to avoid overpaying.

Proof of Concept

As per the description of the project:

On zkSync, the L2 stores the contract's code hashes and not the codes themselves. Therefore, it must be part of the protocol to ensure that no contract with unknown bytecode (i.e. hash with an unknown preimage) is ever deployed.

Also:

If the transaction comes from L2, i.e. (the factory dependencies are yet to publish on L1), the operator prepares the compress the bytecode offchain and then verifies that the bytecode was compressed correctly.

And also:

A call to a contract with invalid bytecode can not be proven. That is why it is essential that no contract with invalid bytecode is ever deployed on zkSync.

In accordance with this description, in the beginning of transaction execution, the bootloader marks bytecode hashes provided by the operator as known. It also validates and sends compressed bytecode to L1. This is done in the ZKSYNC_NEAR_CALL_markFactoryDepsL2 function:

  1. It loads bytecodes provided by the operator and the bytecode hashes provided in the transaction.
  2. It then iterates the hashes provided by the transaction and, for those hashes that match the ones provided by the operator, calls sendCompressedBytecode.
  3. The sendCompressedBytecode takes the bytecodes (bytecode hashes, original bytecodes, and their compressed versions) provided by the operator and passes them to the BytecodeCompressor.publishCompressedBytecode.
  4. publishCompressedBytecode verifies that the bytecodes were compressed correctly and sends them to L1–notice that it sends compressed bytecodes (not original bytecodes or their hashes). It then marks the hashes of the sent bytecodes as known on L2 by calling KNOWN_CODE_STORAGE_CONTRACT.markBytecodeAsPublished.

This aligns with the description of how new contract bytecodes are marked as known and sent to L1 to guarantee that every bytecode hash on L2 has a valid bytecode published on L1.

Now, let's return to ZKSYNC_NEAR_CALL_markFactoryDepsL2 and see what happens after compressed bytecodes were processed:

  1. markFactoryDepsForTx is called with the second argument set to false, meaning that this is not an L1 transaction. The line is accompanied by this comment:

    // For all the bytecodes that have not been compressed on purpose or due to the inefficiency // of compressing the entire preimage of the bytecode will be published. Thus, we expect that the call to markFactoryDepsForTx will publish all uncompressed bytecodes to L1.

  2. markFactoryDepsForTx calls KnownCodesStorage.markFactoryDeps and sets the first argument to true (it toggles the isL1Tx parameter).
  3. markFactoryDeps does three things: validates a bytecode hash, sends bytecode hashes to L1, and marks bytecode hashes as known on L2.
  4. However, markFactoryDeps doesn't send bytecodes (neither original, nor compressed): it only sends bytecode hashes, as can be seen from the code:
    SystemContractHelper.toL1(true, _bytecodeHash, _l1PreimageHash);

This results in a flaw in the bytecodes handling logic: if a bytecode is not compressed and not provided by the operator (i.e. it's not handled by the sendCompressedBytecode function), it won'be sent to L1–only its hash will be sent (KnownCodesStorage.markFactoryDeps sends only bytecode hashes, not bytecodes). Yet it'll still be marked as known on L2. The latter means that, if the operator doesn't provide the bytecode of a contract, it cannot be provided afterwards because the bytecode hash will be known and the bootloader will revert on this line, while trying to publish an already known bytecode. There doesn't seem to be a resolution to this situation, since bytecodes can only be published via the sendCompressedBytecode function. This will break the requirement that all bytecode deployed on the L2 must be published on the L1:

On zkSync, the L2 stores the contract's code hashes and not the codes themselves. Therefore, it must be part of the protocol to ensure that no contract with unknown bytecode (i.e. hash with an unknown preimage) is ever deployed.

And it will also break the prover, since a call to a contract with unknown bytecode cannot be proved.

It's also worth noting that, while the operator is trusted to provide compressed bytecode that's cheaper to send to L1 than sending the original bytecode, the specification doesn't mention that it's also trusted to provide all bytecodes and hashes:

The operator is trusted to provide compressed bytecodes which are advantageous for the users, i.e. it is trusted to make sure that using compression is cheaper than simply publishing the original preimage.

Thus, the operator might not provide all bytecodes required by a transaction, either deliberately or due to a bug.

Tools Used

Manual review

Recommended Mitigation Steps

It seems that the implementation of KnownCodesStorage.markFactoryDeps should publish full bytecodes to L1, since, besides being used to mark hashes as known, it's also calls the _sendBytecodeToL1 function. This is also pointed at by the documentation of the _shouldSendToL1 argument of the internal _markBytecodeAsPublished function:

/// @param _shouldSendToL1 Whether the bytecode should be sent on L1

And also the documentation of the _sendBytecodeToL1 function:

/// @notice Method used for sending the bytecode (preimage for the bytecode hash) on L1.

However, the function only sends bytecode hashes, not bytecodes.

An alternative solutions seems requiring that the operator provides all compresses bytecodes for all factory dependencies of a transactions. This can be implemented as a revert when the bytecode hashes don't match on this line.

To put it more generally, the ZKSYNC_NEAR_CALL_markFactoryDepsL2 function of the bootloader should mark a bytecode hash as known only if:

  1. it's not known;
  2. a. its bytecode (preimage) is provided by the operator and matches the hash (this is not implemented currently);

    b. or, its compressed bytecode is provide by the operator and is verified (this is implemented in the sendCompressedBytecode function of the bootloader);

  3. its compressed or original bytecode is published to L1 (this is currently only implemented for compressed bytecode).
c4-judge commented 1 year ago

GalloDaSballo marked the issue as primary issue

miladpiri commented 1 year ago

The L1 contract will make sure that for every hash published the preimage will be shown as well. In other words, the preimage of the bytecodehash is shown on L1 smart contract. so if the hash is marked as known, the preimage will be publicly known as well.

c4-sponsor commented 1 year ago

miladpiri marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

Closing because Bootloader, the Sponsor has agree with the Staff that they will follow up with the Warden privately

The Warden can follow up with me for any additional question

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Out of scope