c4-bot-6 commented 11 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/x/crosschain/keeper/verify_proof.go#L57-L104

Vulnerability details

Impact

Certain inbound transactions can not be added to the inbound transaction tracker due to the inability to verify their legitimacy, potentially resulting in missed inbound transactions.

Proof of Concept

Anyone can submit inbound transactions to ZetaChain's InTxTracker via the MsgAddToInTxTracker message in case the transaction was missed by the observers in the first place, to ensure the transaction is processed.

The AddToInTxTracker function handles the MsgAddToInTxTracker message and ensures that the submitted transaction is a valid inbound transaction by verifying the provided block inclusion proof in line 28.

15: func (k msgServer) AddToInTxTracker(goCtx context.Context, msg *types.MsgAddToInTxTracker) (*types.MsgAddToInTxTrackerResponse, error) {
..    // [...]
26:     isProven := false
27:     if !(isAdmin || isObserver) && msg.Proof != nil {
28:         txBytes, err := k.VerifyProof(ctx, msg.Proof, msg.ChainId, msg.BlockHash, msg.TxIndex)
29:         if err != nil {
30:             return nil, types.ErrProofVerificationFail.Wrapf(err.Error())
31:         }
32:
33:         if common.IsEVMChain(msg.ChainId) {
34:             err = k.VerifyEVMInTxBody(ctx, msg, txBytes)
35:             if err != nil {
36:                 return nil, types.ErrTxBodyVerificationFail.Wrapf(err.Error())
37:             }
38:         } else {
39:             return nil, types.ErrTxBodyVerificationFail.Wrapf(fmt.Sprintf("cannot verify inTx body for chain %d", msg.ChainId))
40:         }
41:         isProven = true
42:     }
..    // [...]

After verifying if the transaction is included in the block, the transaction is checked if it is a valid inbound transaction by calling the VerifyEVMInTxBody function in line 34.

057: func (k Keeper) VerifyEVMInTxBody(ctx sdk.Context, msg *types.MsgAddToInTxTracker, txBytes []byte) error {
058:    var txx ethtypes.Transaction
059:    err := txx.UnmarshalBinary(txBytes)
060:    if err != nil {
061:        return err
062:    }
063:    if txx.Hash().Hex() != msg.TxHash {
064:        return fmt.Errorf("want tx hash %s, got %s", txx.Hash().Hex(), msg.TxHash)
065:    }
066:    if txx.ChainId().Cmp(big.NewInt(msg.ChainId)) != 0 {
067:        return fmt.Errorf("want evm chain id %d, got %d", txx.ChainId(), msg.ChainId)
068:    }
069:    switch msg.CoinType {
070:    case common.CoinType_Zeta:
071:        coreParams, found := k.zetaObserverKeeper.GetCoreParamsByChainID(ctx, msg.ChainId)
072:        if !found {
073:            return types.ErrUnsupportedChain.Wrapf("core params not found for chain %d", msg.ChainId)
074:        }
075:        if txx.To().Hex() != coreParams.ConnectorContractAddress {
076:            return fmt.Errorf("receiver is not connector contract for coin type %s", msg.CoinType)
077:        }
078:        return nil
079:    case common.CoinType_ERC20:
080:        coreParams, found := k.zetaObserverKeeper.GetCoreParamsByChainID(ctx, msg.ChainId)
081:        if !found {
082:            return types.ErrUnsupportedChain.Wrapf("core params not found for chain %d", msg.ChainId)
083:        }
084:        if txx.To().Hex() != coreParams.Erc20CustodyContractAddress {
085:            return fmt.Errorf("receiver is not erc20Custory contract for coin type %s", msg.CoinType)
086:        }
087:        return nil
088:    case common.CoinType_Gas:
089:        tss, err := k.GetTssAddress(ctx, &types.QueryGetTssAddressRequest{})
090:        if err != nil {
091:            return err
092:        }
093:        tssAddr := eth.HexToAddress(tss.Eth)
094:        if tssAddr == (eth.Address{}) {
095:            return fmt.Errorf("tss address not found")
096:        }
097:        if txx.To().Hex() != tssAddr.Hex() {
098:            return fmt.Errorf("receiver is not tssAddress contract for coin type %s", msg.CoinType)
099:        }
100:        return nil
101:    default:
102:        return fmt.Errorf("coin type %s not supported", msg.CoinType)
103:    }
104: }

The VerifyEVMInTxBody function checks in lines 75 and 84 if the transaction's To address is the expected address for the given coin type. For example, if the coin type is common.CoinType_ERC20, the To address must match the address of the ERC20Custody contract. Otherwise, if the To address is not the expected address, the transaction is considered invalid and not added to the inbound transaction tracker.

However, this check is not sufficient to reliably determine if the transaction is a valid inbound tx.

Specifically, if the ERC20Custody.deposit function was called from another contract, thus the transaction's To address is not the ERC20Custody contract address, the transaction can not be added to the tracker, even though it is a valid inbound transaction. Potentially resulting in a missed inbound transaction.

Similarly, the common.CoinType_Zeta coin type is also affected.

For completeness, I'd like to point out that for the common.CoinType_Gas coin type, if a contract sends native gas tokens to the TSS address, it is not possible to determine if the transaction is a valid inbound transaction.

Tools Used

Manual review

Recommended mitigation steps

Consider also allowing to provide the logged event (log) and the accompanying proof of the event inclusion by verifying the receipt merkle trie proof.

Assessed type

Invalid Validation

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as primary issue

c4-pre-sort commented 11 months ago

DadeKuma marked the issue as sufficient quality report

c4-sponsor commented 11 months ago

lumtis (sponsor) acknowledged

c4-judge commented 11 months ago

0xean marked the issue as satisfactory