c4-bot-3 commented 9 months ago

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/b237708ed5e86f12c4bddabddfd42f001e81941a/repos/node/zetaclient/zeta_supply_checker.go#L221

Vulnerability details

Impact

The Zeta token supply checker delivers false positives, suggesting that there is a Zeta token supply mismatch.

Proof of Concept

The GetPendingCCTXInTransit function in the ZetaSupplyChecker is supposed to return all cctxs that are currently in transit, i.e., cctxs that are pending and soon to be sent to the receiving chains. These pending cctx's are then used to determine the amount of in-transit Zeta tokens (zetaInTransit), subsequently used in the ValidateZetaSupply function to check and validate the Zeta token supply.

Internally, the GetPendingCCTXInTransit function queries all pending cctxs for the receivingChains from the ZetaChain RPC in line 210. Thereafter, the cctxs are filtered in lines 221-233 by removing all cctxs that have been added to the OutTxTracker.

207: func (zs *ZetaSupplyChecker) GetPendingCCTXInTransit(receivingChains []common.Chain) []*types.CrossChainTx {
208:    cctxInTransit := make([]*types.CrossChainTx, 0)
209:    for _, chain := range receivingChains {
210:        cctx, err := zs.zetaClient.GetAllPendingCctx(chain.ChainId)
211:        if err != nil {
212:            continue
213:        }
214:        nonceToCctxMap := make(map[uint64]*types.CrossChainTx)
215:        for _, c := range cctx {
216:            if c.GetInboundTxParams().CoinType == common.CoinType_Zeta {
217:                nonceToCctxMap[c.GetCurrentOutTxParam().OutboundTxTssNonce] = c
218:            }
219:        }
220:
221:        trackers, err := zs.zetaClient.GetAllOutTxTrackerByChain(chain, Ascending)
222:        if err != nil {
223:            continue
224:        }
225:        for _, tracker := range trackers {
226:            zs.logger.Info().Msgf("tracker exists for nonce: %d , removing from supply checks", tracker.Nonce)
227:            delete(nonceToCctxMap, tracker.Nonce)
228:        }
229:        for _, c := range nonceToCctxMap {
230:            if c != nil {
231:                cctxInTransit = append(cctxInTransit, c)
232:            }
233:        }
234:    }
235:    return cctxInTransit
236: }

The reasoning for this is that once a cctx has been broadcasted to the receiver chain by the observers, the cctx is added to the OutTxTracker. As a result, the cctx is no longer considered to be in transit, the Zeta token supply on the receiver chain is updated (via minting or unlocking) and the update Zeta token supply is either included in the ethLockedAmount or externalChainTotalSupply.

However, the assumption that when a cctx is broadcasted to the receiver chain and included in the OutTxTracker, the cctx is no longer in transit is only partially correct. Once the transaction is broadcasted, it first stays in the EVM mempool until it is included in a block. Only then, the transaction is executed in the EVM, the state transition is applied and the Zeta token supply updated.

Consequently, the in-transit Zeta token is not correctly tracked, and the supply checker can not correctly validate the Zeta token supply.

Tools Used

Manual review

Recommended mitigation steps

I'm not entirely sure how to fix this issue and if it's even viable to do so. Rather, the supply checker should be allowed to work with a margin of error, i.e., the supply checker should be allowed to be off by a certain amount of Zeta tokens.

Assessed type

Other

c4-pre-sort commented 9 months ago

DadeKuma marked the issue as primary issue

DadeKuma commented 9 months ago

Pending transactions in mempool might break the max supply invariant

c4-pre-sort commented 9 months ago

DadeKuma marked the issue as sufficient quality report

c4-sponsor commented 9 months ago

lumtis (sponsor) confirmed

c4-judge commented 8 months ago

0xean marked the issue as satisfactory