code-423n4 / 2023-11-zetachain-findings

0 stars 0 forks source link

ZetaConnectorZEVM's send() lacks reentrancy protection, risking token loss and corruption. #38

Closed c4-submissions closed 11 months ago

c4-submissions commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/44c8dd426e829536850b5d42b3f0ade1ce29a23c/repos/protocol-contracts/contracts/zevm/ZetaConnectorZEVM.sol#L92-L108

Vulnerability details

Impact

The send() function makes 3 external contract calls to WZETA and FUNGIBLE_MODULE_ADDRESS without locking state first via a reentrancy guard. This allows an attacker to make a callback back into send() before the first call finishes.

For example, the WZETA.withdraw() call could trigger a malicious WZETA contract to call send() again. This second invocation would then operate on the same storage state as the first, leading to potentially unintended side effects.

So, the external calls made in ZetaConnectorZEVM's send() function can be exploited by a malicious callback contract to re-enter send() before the first invocation finishes. This can lead to:

Proof of Concept

As we can see, the send() function makes external calls to WZETA.transferFrom(), WZETA.withdraw(), and FUNGIBLE_MODULE_ADDRESS.call() without locking state first. This allows for a reentrancy attack.

    function send(ZetaInterfaces.SendInput calldata input) external {
        // Transfer wzeta to "fungible" module, which will be burnt by the protocol post processing via hooks.
        if (!WZETA(wzeta).transferFrom(msg.sender, address(this), input.zetaValueAndGas)) revert WZETATransferFailed();
        WZETA(wzeta).withdraw(input.zetaValueAndGas);
        (bool sent, ) = FUNGIBLE_MODULE_ADDRESS.call{value: input.zetaValueAndGas}("");
        if (!sent) revert FailedZetaSent();
        emit ZetaSent(
            tx.origin,
            msg.sender,
            input.destinationChainId,
            input.destinationAddress,
            input.zetaValueAndGas,
            input.destinationGasLimit,
            input.message,
            input.zetaParams
        );
    }

Tools Used

Manual review

Recommended Mitigation Steps

Add a [Re-entrancy Guard] to the function. The function should use a Checks-Effects-Interactions pattern. The external calls should be executed at the end of the function and all the state-changing must happen before the call.

Assessed type

Reentrancy

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as insufficient quality report

DadeKuma commented 11 months ago

QA

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as primary issue

c4-judge commented 11 months ago

0xean marked the issue as unsatisfactory: Insufficient quality

c4-judge commented 10 months ago

0xean marked the issue as unsatisfactory: Insufficient quality