code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-03-zksync/blob/main/contracts/EventWriter.yul#L114-L126

Vulnerability details

Impact

It was clearly indicated by the sponsor in the discord discussion channel that the EventWriter.yul is in the scope, although it is not listed on the contest details page.

EventWriter lacks callvalue check. The value sent to EventWriter will stuck in the contract.

Proof of Concept

Yul doesn't have the payable modifier. It should check if the callvalue() is zero to ensure that no value is sent to a contract address which can't transfer any token out. But the EventWriter lacks the check.

            ////////////////////////////////////////////////////////////////
            //                      FALLBACK
            ////////////////////////////////////////////////////////////////

            // Ensure that contract is called on purpose
            onlySystemCall()

            let numberOfTopics := getExtraAbiData_0()
            // Only 4 indexed fields are allowed, same as on EVM
            if gt(numberOfTopics, 4) {
                revert(0, 0)
            }

            let dataLength := calldatasize()

Tools Used

Manual review

Recommended Mitigation Steps

Add the check:

if iszero(iszero(callvalue())) {
    revert(0, 0)
}

miladpiri commented 1 year ago

The value may stuck on precompiles as well, it a not a bug so.

c4-sponsor commented 1 year ago

miladpiri marked the issue as sponsor disputed

GalloDaSballo commented 1 year ago

I believe this meets the requirements for "funds stuck", so I'm marking as QA Low

That said, I agree with a nofix and rationally people should not send funds to the EventWriter

GalloDaSballo commented 1 year ago

L

c4-judge commented 1 year ago

GalloDaSballo changed the severity to QA (Quality Assurance)