Users pay excessive gas cost for sending bytecode hashes to L1

[code423n4]

Lines of code

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

Vulnerability details

Impact

When deploying new contracts, users are forced to pay more gas than is required to publish the bytecode hash to the L1.

Proof of Concept

When users deploy new smart contracts, the protocol marks the hashes of the bytecodes of the contracts as known and sends the bytecode to the L1. As per the description of the project:

On zkSync, the L2 stores the contract's code hashes and not the codes themselves.

And:

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. After that, we send the L2→L1 log with the compressed bytecode of the contract. It is the responsibility of the L1 contracts to verify that the corresponding bytecode hash has been published on L1.

There are two functions that handle publishing of bytecodes and their hashes to the L1 network:

1. BytecodeCompressor.publishCompressedBytecode validates compressed bytecode and publishes it to the L1. The functions calls the L1Messenger.sendToL1 method to send the bytecode to the L1. Inside sendToL1, the amount of the gas that the transaction sender needs to pay is computed as:

   uint256 pubdataLen;
   unchecked {
       pubdataLen = ((_message.length + 31) / 32) * 32 + 64;
   }
   uint256 gasToPay = pubdataLen * gasPerPubdataBytes;

   I.e. the user sends the rate of gasPerPubdataBytes per each byte of the sent data (that is, the bytecode). Actual publishing of bytecode is implemented by emitting a message:

   emit L1MessageSent(msg.sender, hash, _message);

   To put it simply, the user pays for the size of the bytecode.

2. KnownCodesStorage.markFactoryDeps is used to publish bytecode hashes to the L1. The bootloader always calls this function when processes transaction factory dependencies. markFactoryDeps calls _markBytecodeAsPublished, which in its turn calls _sendBytecodeToL1 to handle publishing. Inside _sendBytecodeToL1 the cost of publishing is computed identically to BytecodeCompressor.publishCompressedBytecode (+ the overhead cost):

   // Get the cost of 1 pubdata byte in gas
   uint256 meta = SystemContractHelper.getZkSyncMetaBytes();
   uint256 pricePerPubdataByteInGas = SystemContractHelper.getGasPerPubdataByteFromMeta(meta);
   
   uint256 gasToPay = (_l1PreimageBytesLen + BYTECODE_PUBLISHING_OVERHEAD) * pricePerPubdataByteInGas;

   Where _l1PreimageBytesLen is the length of the bytecode (not the hash). However, _sendBytecodeToL1 doesn't emit a L1MessageSent log with a full bytecode: similarly to L1Messenger.sendToL1, it calls SystemContractHelper.toL1, but it doesn't emit a L1MessageSent event after that.

Thus, the KnownCodesStorage._sendBytecodeToL1 function charges users for publishing the entire bytecode while publishing only its hash.

Tools Used

Manual review

Recommended Mitigation Steps

In the KnownCodesStorage._sendBytecodeToL1 consider computing the amount of gas to burn based on the actual amount of bytes published to the L1. Judging by the fact that the functions only sends a bytecode hash, users should only be charge to pay for the size of a hash, not the size of an entire bytecode.

[c4-judge]

GalloDaSballo marked the issue as primary issue

[GalloDaSballo]

Primary for detail

[c4-sponsor]

vladbochok marked the issue as sponsor disputed

[vladbochok]

Operator actually sends the whole bytecode to the L1, otherwise it will be impossible to restore chain observing the L1.

[GalloDaSballo]

@vladbochok to confirm, you're disputing because the sender must pay for the entire cost, since zkSync / operator will have to publish the entire code, meaning the deployer should pay the entire cost to offset the operator cost?

[vladbochok]

@GalloDaSballo exactly! The L1 contract enforces the publishing preimage from the bytecode hash

[GalloDaSballo]

Per the discussion with the sponsor, the cost is paid to offset the operator cost

[c4-judge]

GalloDaSballo marked the issue as unsatisfactory: Invalid