Multiple events in the same TX cause loss of funds
Description
When a user sends ZETA or ERC-20 tokens from an EVM chain to another chain, events are emitted on the EVM from specific contracts that are picked up by the ZetaChain observers. The observers then execute the VoteOnObservedInboundTx()
message on ZetaChain with the details of the event. This is done through a ballot system, where the intention is for each event to have its own ballot.
The ballots are uniquely identified by their index, and the index for each ballot in this case is a hash of the VoteOnObservedInboundTx
message (with the Creator
set to ""
). If a ballot is not found for a specific hash, then it is created.
func (k msgServer) VoteOnObservedInboundTx(goCtx context.Context, msg *types.MsgVoteOnObservedInboundTx)
(/*...*/) {
// [ ... ]
index := msg.Digest()
// Add votes and Set Ballot
// GetBallot checks against the supported chains list before querying for Ballot
ballot, isNew, err := k.zetaObserverKeeper.FindBallot(ctx, index, observationChain, observationType)
if err != nil {
return nil, err
}
// [ ... ]
}
The MsgVoteOnObservedInboundTx
structure is as follows:
type MsgVoteOnObservedInboundTx struct {
Creator string
Sender string
SenderChainId int64
Receiver string
ReceiverChain int64
Amount github_com_cosmos_cosmos_sdk_types.Uint
Message string
InTxHash string
InBlockHeight uint64
GasLimit uint64
CoinType common.CoinType
TxOrigin string
Asset string
}
Impact
Looking closely at the above structure, if multiple events are emitted in the same transaction with the same Receiver
, Amount
, and Message
, the hash for every event would be the same. These events are usually uniquely identified by their log index in the transaction logs.
This leads to an issue where a smart contract on the EVM might trigger multiple cross-chain transfers in the same transaction but only one ballot for the very first event in the transaction. All subsequent calls to VoteOnObservedInboundTx()
for each event will lead to the same ballot being voted on, rather than a new ballot being created for each event.
This would cause the sender to lose access to their funds.
An easy way to trigger this bug on the local developer network is to deploy a smart contract like the following on EVM (note the code is incomplete):
contract Test {
ERC20Custody custody =
ERC20Custody(0xD28D6A0b8189305551a0A8bd247a6ECa9CE781Ca);
IERC20 usdt = IERC20(0xff3135df4F2775f4091b81f4c7B6359CfA07862a);
address deployer = 0xE5C5367B8224807Ac2207d350E60e1b6F27a7ecC;
function runTest() external {
usdt.approve(address(custody), type(uint256).max);
usdt.transferFrom(deployer, address(this), 10e6);
for (int i = 0; i < 10; i++) {
custody.deposit(abi.encodePacked(deployer), usdt, 1e6, "");
}
}
}
Initially, the deployer on the zEVM will have zero USDTZRC20 tokens. After calling runTest()
, the deployer should have 10e6 USDTZRC20 tokens if everything works out correctly. In reality, the deployer will only have 1e6 tokens, as all the events emitted will generate the same hash and thus only the very first event will be processed.
Recommendations
We reported a similar issue previously, which applied to zEVM-to-EVM communication. That issue was fixed by adding each event's log index to the gas limit within the MsgVoteOnObservedInboundTx
message.
Ideally, the message should be modified to contain a log-index field, but if that is not possible, the gasLimit + event.Raw.Index
fix should be applied in the evm_client
, within the GetInboundVoteMsgForZetaSentEvent()
and GetInboundVoteMsgForDepositedEvent()
.
Remediation
ZetaChain implemented a fix for this issue in Pull Request #1372↗. Specifically, the event index was implemented for inbound tx digests.