Potential Reentrancy through Gas Fee Transfer in `withdraw()` function

[c4-bot-5]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/protocol-contracts/contracts/zevm/ZRC20.sol#L256-L264

Vulnerability details

Summary

The withdraw function in ZRC20.sol is vulnerable to reentrancy attacks due to improper code ordering. The withdraw function transfers gas fee tokens to the Fungible Module contract before burning

Vulnerability Details

The withdraw() function first calls transferFrom() to pay gas fees, then calls _burn() to reduce the balance:

    function withdraw(bytes memory to, uint256 amount) external override returns (bool) {
        (address gasZRC20, uint256 gasFee) = withdrawGasFee();
        if (!IZRC20(gasZRC20).transferFrom(msg.sender, FUNGIBLE_MODULE_ADDRESS, gasFee)) {
            revert GasFeeTransferFailed();
        }
        _burn(msg.sender, amount);
        emit Withdrawal(msg.sender, to, amount, gasFee, PROTOCOL_FLAT_FEE);
        return true;
    }

This allows transferFrom() to call withdraw again before _burn() is executed.

Impact

The impact would likely only drain extra gas fee tokens rather than withdraw amounts. Risk depends on assuming benign behavior of Fungible Module.

Proof of Concept

Tools Used

Manual Review

Recommended Mitigation Steps

Apply checks-effects-interactions pattern:

function withdraw(uint amount) nonReentrant external {
  _burn(msg.sender, amount);  
  token.transferFrom(msg.sender, address(this), fee);
  emit Event();
}

Burn tokens first before external calls. Use OpenZeppelin nonReentrant modifier to prevent reentrancy.

Assessed type

Reentrancy

[c4-pre-sort]

DadeKuma marked the issue as insufficient quality report

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #38

[c4-judge]

0xean marked the issue as unsatisfactory: Insufficient quality