Any calls to 0xa457c2d7 and 0xa457c2d7 function selector will be reverted globally for any contract

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/evm_hooks.go#L52-L71

Vulnerability details

Whenever etheremint processes a transaction, it calls EVM hook called PostTxProcessing in Cosmos. It's a way to give developers a way to react to the changes in EVM. ZetaChain uses it to process events emited by the contracts and catch Zeta withdrawals and outbound transactions. It has additional check, however, making sure that following function selectors are not called:

• 0x39509351 - increaseAllowance(address,uint256)
• 0xa457c2d7 - decreaseAllowance(address,uint256)

It can be seen in the following code:

// PostTxProcessing implements EvmHooks.PostTxProcessing.
func (k Keeper) PostTxProcessing(
    ctx sdk.Context,
    msg core.Message,
    receipt *ethtypes.Receipt,
) error {
    abiStr := "[{\"inputs\":[{\"internalType\":\"string\",\"name\":\"name_\",\"type\":\"string\"}," +
  // [...]
    inputData := msg.Data()
    if len(inputData) >= 4 {
        // Check if method exist in ABI
        methodID := inputData[:4]
        parsedABI, err := abi.JSON(strings.NewReader(abiStr))
        if err != nil {
            return err
        }
        for _, method := range parsedABI.Methods {
            if bytes.Equal(methodID, method.ID) {
                // Check if deactivated method
                if method.Name == "increaseAllowance" || method.Name == "decreaseAllowance" {
                    return fmt.Errorf("%s not allowed", method.Name)
                }
            }
        }
    }

    var emittingContract ethcommon.Address
    if msg.To() != nil {
        emittingContract = *msg.To()
    }
    return k.ProcessLogs(ctx, receipt.Logs, emittingContract, msg.From().Hex())
}

As you can see the check is implemented improperly, and I assume that the intention was to block ZRC20 only related functionalities. However, the code actually doesn't check recipient of the call. That means that any smart contract call starting with 0x39509351 or 0xa457c2d7 will return an error, reverting whole transactions. This has grave impact if any of omnichain projects decides to move to Zeta and has function selectors clashing with the ones listed. That would have grave consequences, DoS specific functionalities of the contracts.

There is additional note here. Because msg.Data consist of transaction calldata, it can be easily circumvented by using intermediary smart contract that internally calls functions with those selectors.

Impact

DoSing valid transactions with 0x39509351 or 0xa457c2d7 function selectors.

Proof of Concept

1. New protocol PumpNDumpSwap deploys smart contracts with function having it's core function's selector as 0x39509351.
2. Millions in liquidity is seeded on ZetaChain.
3. The protocol starts operating, however it's unusable because transactions starting with 0x39509351 are reverted in PostTxProcessing. The funds are locked in the contract.

Tools Used

Manual analysis

Recommended Mitigation Steps

Block increaseAllowance() and decreaseAllowance() directly in ZRC20, and remove the check from PostTxProcessing hook.

Assessed type

DoS

[c4-pre-sort]

DadeKuma marked the issue as duplicate of #276

[c4-pre-sort]

DadeKuma marked the issue as sufficient quality report

[c4-judge]

0xean marked the issue as unsatisfactory: Out of scope