Compilation of string literals longer than 32 bytes in `Bootloader` succeeds without error

[c4-submissions]

Lines of code

https://github.com/code-423n4/2023-10-zksync/blob/ebec640786cf3cb791fa77a856ad9d2a554dad3f/code/system-contracts/bootloader/bootloader.yul#L2516

Vulnerability details

Impact

The Bootloader contract doesn't compile as expected. It silently omits the string literal from the compiled bytecode, potentially resulting in erroneous execution of the code within the Bootloader and the Operator's software.

Proof of Concept

Yul doesn't support string literals longer than 32 bytes.

The publishTimestampDataToL1() function uses a 35-byte string, Failed publish timestamp data to L1, to call the debugLog() function, which will indicate the error to the Operator.

https://github.com/code-423n4/2023-10-zksync/blob/ebec640786cf3cb791fa77a856ad9d2a554dad3f/code/system-contracts/bootloader/bootloader.yul#L2501-L2519

Typically, in the original Solidity (solc) compiler, this would trigger a compile error. However, in zksolc, this error is ignored, and the code compiles without any issues; you could try to compile the following code with solc:

contract Test {
    function returnString() public view returns (string memory a) {
        assembly {
            a := "Failed publish timestamp data to L1" // TypeError: String literal too long (35 > 32)
        }
    }
}

To reproduce the issue:

1. Execute the provided one-liner to set up dependencies and compile the Bootloader:

   rm -Rf 2023-10-zksync || true && git clone https://github.com/code-423n4/2023-10-zksync.git && cd 2023-10-zksync/code/system-contracts/scripts && yarn --ignore-engines && bash quick-setup.sh

2. Access the ./code/system-contracts/bootloader/build/proved_batch.yul.zbin file, which contains the compiled bytecode for production use.
3. Attempt to locate the string Failed publish timestamp data to L1 within the bytecode. You will not find it. However, if you search for another string, for example, Failed to publish L2Logs data, you will see it in the bytecode.

This problem mainly pertains to the differences between zksolc and solc. It has the potential to impact all contracts deployed on zkSync, including user contracts that utilize Assembly blocks or Yul. However, it's important to note that the compiler itself is not within the scope of this contest.

A Medium rating is appropriate for this issue based on its specific context, considering that the affected code segment is unlikely to be frequently executed. However, it's important to consider assigning a High rating due to the potential risk it poses to user smart contracts.

A similar problem was identified in a prior audit by OZ, where it was categorized as a Medium-level issue. https://blog.openzeppelin.com/zksync-bootloader-audit-report#string-literal-exceeds-32-bytes-limit

Tools Used

Manual review, zksolc-windows-amd64-gnu-v1.3.14.exe

Recommended Mitigation Steps

Shorten the string and add additional checks to the zksolc compiler.

Assessed type

Other

[c4-pre-sort]

bytes032 marked the issue as primary issue

[c4-pre-sort]

bytes032 marked the issue as low quality report

[miladpiri]

Low/QA. The issue exists, but the impact is very minor.

[c4-sponsor]

miladpiri marked the issue as disagree with severity

[c4-sponsor]

miladpiri (sponsor) confirmed

[c4-judge]

GalloDaSballo changed the severity to QA (Quality Assurance)