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

4 stars 0 forks source link

Potential Gas Manipulation via Bytecode Compression #71

Open c4-submissions opened 1 year ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Compressor.sol#L54

Vulnerability details

Impact

Malicious operators could exploit this issue to overcharge users by artificially increasing the length of the dictionary without any benefit to the encoding process. As a result, users may end up paying higher gas costs for message publication in L1, leading to an adverse financial impact. This issue undermines the intended efficiency and cost-effectiveness of the compression mechanism.

Proof of Concept

When processing L2 transactions, it is essential to mark the user's provided factoryDeps on L2 and subsequently publish them to L1. bootloader::processL2Tx >> bootloader::l2TxExecution >> bootloader::ZKSYNC_NEAR_CALL_markFactoryDepsL2 >> bootloader::sendCompressedBytecode >> Compressor::publishCompressedBytecode https://github.com/code-423n4/2023-10-zksync/blob/main/code/system-contracts/contracts/Compressor.sol#L54

This process involves compression to reduce gas consumption during publishing to L1. The sequence of actions are as follows:

  1. The remaining gas allocated by the user is utilized to initiate the publication of factoryDeps. https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/bootloader/bootloader.yul#L1242

  2. Compressed data is sent to L1. https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/contracts/Compressor.sol#L79

  3. Gas consumption is determined by the length of the transmitted message. https://github.com/code-423n4/2023-10-zksync/blob/72f5f16ed4ba94c7689fe38fcb0b7d27d2a3f135/code/system-contracts/contracts/L1Messenger.sol#L153

However, this process could be exploited by a malicious operator to overcharge users. Through manipulation of the compression method, the operator could inflate gas costs. In this context, not only may the compressed bytecode fail to be shorter than the original, but it could even become longer.

To illustrate, consider an example where the bytecode to be compressed and published is ABCB (with each character representing 8 bytes). Notably, the second and fourth 8-byte segments are identical.

In an ideal compression scenario, the _rawCompressedData would appear as: 0x0003ABC0000000100020001. Here, ABC forms the dictionary, and encodedData is 0x0000000100020001. The prefix 0x0003 indicates the dictionary's length in 8-byte segments, while the encodedData references the dictionary's segments in this order: 0, 1, 2, 1, which corresponds to A, B, C, and B, resepectively.

However, a malicious operator, could artificially extend the dictionary length. They might modify _rawCompressedData to be: 0x0004ABCX0000000100020001. In this scenario, ABCX constitutes the dictionary, while encodedData remains 0x0000000100020001. This essentially introduces an extra 8-byte X to the dictionary, which serves no functional purpose, just increases the dictionary length. The encodedData still references the same segments, 0, 1, 2, 1, without employing the added X.

In summary, this manipulation increases the dictionary's length by appending an unnecessary chunk, while not functional, this lengthening of the dictionary results in higher charges for users. Importantly, the dictionary remains valid, as it remains possible to decode the original bytecode from _rawCompressedData using the encodedData.

Tools Used

Recommended Mitigation Steps

The function publishCompressedBytecode should be revised as follows, where an array named usedDictionaryIndex is introduced to monitor the utilization of dictionary chunks. Subsequently, it validates whether all chunks in the dictionary have been utilized.

function publishCompressedBytecode(
        bytes calldata _bytecode,
        bytes calldata _rawCompressedData
    ) external payable onlyCallFromBootloader returns (bytes32 bytecodeHash) {
        unchecked {
            (bytes calldata dictionary, bytes calldata encodedData) = _decodeRawBytecode(_rawCompressedData);

            require(dictionary.length % 8 == 0, "Dictionary length should be a multiple of 8");
            require(dictionary.length <= 2 ** 16 * 8, "Dictionary is too big");
            require(
                encodedData.length * 4 == _bytecode.length,
                "Encoded data length should be 4 times shorter than the original bytecode"
            );

            // This code is added
            bool[] memory usedDictionaryIndex = new bool[](
                dictionary.length / 8
            );
            //////////////////////

            for (uint256 encodedDataPointer = 0; encodedDataPointer < encodedData.length; encodedDataPointer += 2) {
                uint256 indexOfEncodedChunk = uint256(encodedData.readUint16(encodedDataPointer)) * 8;
                require(indexOfEncodedChunk < dictionary.length, "Encoded chunk index is out of bounds");

                // This code is added
                usedDictionaryIndex[indexOfEncodedChunk] = true;
                //////////////////////

                uint64 encodedChunk = dictionary.readUint64(indexOfEncodedChunk);
                uint64 realChunk = _bytecode.readUint64(encodedDataPointer * 4);

                require(encodedChunk == realChunk, "Encoded chunk does not match the original bytecode");
            }

            // This code is added
            for (uint256 i = 0; i < usedDictionaryIndex.length; ++i) {
                require(
                    usedDictionaryIndex[i],
                    "the dictionary includes chunks that are uselss"
                );
            }
            //////////////////////
        }

        bytecodeHash = Utils.hashL2Bytecode(_bytecode);
        L1_MESSENGER_CONTRACT.sendToL1(_rawCompressedData);
        KNOWN_CODE_STORAGE_CONTRACT.markBytecodeAsPublished(bytecodeHash);
    }

Assessed type

Context

c4-pre-sort commented 1 year ago

bytes032 marked the issue as sufficient quality report

miladpiri commented 1 year ago

Valid report. This report is about providing useless long compressed data leading to consume user’s gas more (if it consumes much gas, nothing is left for the execution, so it will fail).

c4-sponsor commented 1 year ago

miladpiri (sponsor) confirmed

c4-judge commented 12 months ago

GalloDaSballo marked the issue as primary issue

c4-judge commented 12 months ago

GalloDaSballo marked the issue as selected for report

GalloDaSballo commented 12 months ago

The Warden has shown a way for the Operator to burn excess gas as a strategy to maximize profits, since this directly conflicts with the refund system, Medium Seems appropriate