code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Injection of duplicate code.

Proof of Concept

In the _markBytecodeAsPublished function, the contract does not check whether a particular hash has already been added to the storage.

The contract only checks the current marker value of the hash, which is set to 0 by default, and if the value is still 0, the hash is saved to the storage, and the contract emits an event MarkedAsKnown. This means that an attacker can replay a transaction with the same bytecode hash, and the contract will add the same hash again to the storage, resulting in duplicate code.

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

Tools Used

Manual audit

Recommended Mitigation Steps

The contract should first check whether the hash has already been added to the storage before saving it again.

Updated code:

function _markBytecodeAsPublished(
    bytes32 _bytecodeHash,
    bytes32 _l1PreimageHash,
    uint256 _l1PreimageBytesLen,
    bool _shouldSendToL1
) internal {
    if (getMarker(_bytecodeHash) == 0) {
        _validateBytecode(_bytecodeHash);

        if (_shouldSendToL1) {
            _sendBytecodeToL1(_bytecodeHash, _l1PreimageHash, _l1PreimageBytesLen);
        }

        // Save as known, to not resend the log to L1
        assembly {
            sstore(_bytecodeHash, 1)
        }

        emit MarkedAsKnown(_bytecodeHash, _shouldSendToL1);
+   } else {
+       emit AlreadyMarkedAsKnown(_bytecodeHash);
    }
}

GalloDaSballo commented 1 year ago

If the hash is known nothing happens

c4-judge commented 1 year ago

GalloDaSballo marked the issue as unsatisfactory: Insufficient quality

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