Submitted by ChristiansWhoHack, also found by ChristiansWhoHack
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/zetaclient/inbound_tracker.go#L144 
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/zetaclient/inbound_tracker.go#L179 
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/zetaclient/inbound_tracker.go#L213 
https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/zetaclient/inbound_tracker.go#L71
When utilizing state from multiple blockchains, it is important that this information is finalized. If not, a block reorganization can occur. This is possible since some subset of nodes may have one chain but the other set will have another. When this happens, a double spend can occur. For instance, if node set A includes the transaction and is processed as the event occurring but then a block reorganization occurs going to node set B that does NOT include a transaction, then the funds were processed prematurely. Now, the user has funds in both locations. For more on this, read the post by Halborn. This finding is similar to the Outbound Block confirmation check missing but is different because there are in separate sections of code and one is outbound while the other is inbound (this one).
Zetachain has set amount of blocks that it must observe to be completed before it will be confirmed that a block occurred or not. After this threshold of new blocks has been met, it is presumed that no block reorganization will occur. In previous audits, the Bitcoin confirmation count was too low, leading to potential of double spend issues. When processing incoming transaction through the normal event handling, Zetachain verifies that enough blocks have passed.
It is possible to bypass the confirmation protection by using the InTxTracker functionality. The confirmation checks are completely missing for both Ethereum and Bitcoin clients when performing these event checks. Directly after seeing the event in the TxTracker queue, the events will be queried from the blockchain without verifying the amount of blocks that have passed. The regular event handling and TxTracker processing are implemented completely separately, even though they are very similar in nature.
To exploit this, the attacker must be an observer. This is because untrusted relayers must add in a proof, which requires the header to be at a certain point. In particular, the confirmation check is respected on the block header voting. However, an observer does not have to provide a proof; they can simply provide by TxHash and it will be added to the functionality. The observers are somewhat trusted entities but this is still a decentralized system. A malicious or compromised observer should not be able to forgo the confirmation checks entirely to lead the environment vulnerable to a double spend attack.
The result of this is that a malicious observer/compromised node could see a fork happening, add this to the Tracker, then profit from the reorganization that happens later. This would duplicate the funds; once on the specified destination from the CCTX and one in the owners wallet. Practically exploiting this is difficult though.
Actually implementing an exploit that causes a double spend by a block reorganization is extremely difficult to do in a test environment. As a result, we deemed that a sufficient way to write a proof of concept for this is to demonstrate that the CCTXs are being operated on even though not enough blocks have been passed. From this, it is easy to deduce that a block reorganization would result in stolen funds.
To prepare for the exploit, do the following:
Respect the block confirmation limits on both Ethereum and Bitcoin for inbound transactions. Simply ensure that enough blocks have passed prior to acting on them.
This is done properly within the ObserveTxIn code but not in the InboundTracker code.
The code between the InboundTracker and ObserveTxIn does almost the exact same parsing. The only difference is that the InboundTracker is told which transactions to look at while the latter looks at all transactions. The original was safe but the newer code is not.
To prevent these sorts of issues in the future, try to share the code between the two locations. This way, security issues are not repeated over and over again.
lumtis (ZetaChain) confirmed, but disagreed with severity and commented:
0xean (Judge) decreased severity to Medium
