From 9f6239b6f20df6ed1fad09af02fd62bfa58cf7bb Mon Sep 17 00:00:00 2001 From: Masih Yeganeh Date: Mon, 23 Feb 2026 00:46:23 +0330 Subject: [PATCH] Fix DEX freeze bug --- integration-tests/modules/dex_test.go | 193 ++++++++++++++++++++++++++ x/asset/ft/keeper/keeper_dex.go | 38 +++++ x/asset/ft/keeper/keeper_dex_test.go | 113 +++++++++++++++ 3 files changed, 344 insertions(+) diff --git a/integration-tests/modules/dex_test.go b/integration-tests/modules/dex_test.go index 2ff63c57..40b12560 100644 --- a/integration-tests/modules/dex_test.go +++ b/integration-tests/modules/dex_test.go @@ -3046,3 +3046,196 @@ func assertBalance( require.NoError(t, err) require.Equal(t, sdkmath.NewInt(expectedBalance).String(), acc1Denom2BalanceRes.Balance.Amount.String()) } + +// TestFrozenTokenEscapeViaDEX verifies that frozen tokens cannot be DEX-locked +// or transferred via DEX settlement. +// +// This is the integration test counterpart of TestKeeper_FrozenTokenEscapeViaDEX. +// If the fix were missing: placing multiple sell orders could lock frozen tokens +// (DEXIncreaseLocked had no frozen check), and DEX settlement would transfer them +// via raw bank keeper. This test asserts the fix: the second order (which would +// lock frozen tokens) must be rejected. +func TestFrozenTokenEscapeViaDEX(t *testing.T) { + t.Parallel() + ctx, chain := integrationtests.NewTXChainTestingContext(t) + + requireT := require.New(t) + assetFTClient := assetfttypes.NewQueryClient(chain.ClientContext) + dexClient := dextypes.NewQueryClient(chain.ClientContext) + + dexParamsRes, err := dexClient.Params(ctx, &dextypes.QueryParamsRequest{}) + requireT.NoError(err) + + // ======================================================================== + // SETUP: Create token with Feature_freezing, fund Alice, freeze tokens + // ======================================================================== + + // Issue base token with freezing feature (the token Alice sells) + // issuer already has the initial amount from issuance + issuer, denom := genAccountAndIssueFT( + ctx, t, chain, 1_000_000, sdkmath.NewInt(1_000_000), assetfttypes.Feature_freezing, + ) + + // Issue quote token (the token Bob uses to buy) - issuer issues both tokens + quoteDenom := issueFT(ctx, t, chain, issuer, sdkmath.NewInt(1_000_000)) + + alice := chain.GenAccount() + bob := chain.GenAccount() + + // Fund accounts with necessary messages and amounts + chain.FundAccountsWithOptions(ctx, t, []integration.AccWithBalancesOptions{ + { + Acc: issuer, + Options: integration.BalancesOptions{ + Messages: []sdk.Msg{ + &banktypes.MsgSend{}, + &banktypes.MsgSend{}, + &assetfttypes.MsgFreeze{}, + }, + }, + }, + { + Acc: alice, + Options: integration.BalancesOptions{ + Amount: dexParamsRes.Params.OrderReserve.Amount.MulRaw(3).Add(sdkmath.NewInt(100_000)), + }, + }, + { + Acc: bob, + Options: integration.BalancesOptions{ + Amount: dexParamsRes.Params.OrderReserve.Amount.Add(sdkmath.NewInt(100_000)), + }, + }, + }) + + // Fund Alice with 1,000,000 base tokens (100 * 10,000 to meet quantity step requirement) + aliceBaseAmount := int64(1_000_000) + sendMsg := &banktypes.MsgSend{ + FromAddress: issuer.String(), + ToAddress: alice.String(), + Amount: sdk.NewCoins(sdk.NewInt64Coin(denom, aliceBaseAmount)), + } + _, err = client.BroadcastTx( + ctx, + chain.ClientContext.WithFromAddress(issuer), + chain.TxFactory().WithGas(chain.GasLimitByMsgs(sendMsg)), + sendMsg, + ) + requireT.NoError(err) + + // Fund Bob with 1,000,000 quote tokens (for buying) + bobQuoteAmount := int64(1_000_000) + bobSendMsg := &banktypes.MsgSend{ + FromAddress: issuer.String(), + ToAddress: bob.String(), + Amount: sdk.NewCoins(sdk.NewInt64Coin(quoteDenom, bobQuoteAmount)), + } + _, err = client.BroadcastTx( + ctx, + chain.ClientContext.WithFromAddress(issuer), + chain.TxFactory().WithGas(chain.GasLimitByMsgs(bobSendMsg)), + bobSendMsg, + ) + requireT.NoError(err) + + // Admin freezes 600,000 of Alice's base tokens (60 * 10,000) + frozenAmount := int64(600_000) + freezeMsg := &assetfttypes.MsgFreeze{ + Sender: issuer.String(), + Account: alice.String(), + Coin: sdk.NewInt64Coin(denom, frozenAmount), + } + _, err = client.BroadcastTx( + ctx, + chain.ClientContext.WithFromAddress(issuer), + chain.TxFactory().WithGas(chain.GasLimitByMsgs(freezeMsg)), + freezeMsg, + ) + requireT.NoError(err) + + // Verify initial state + balanceRes, err := assetFTClient.Balance(ctx, &assetfttypes.QueryBalanceRequest{ + Account: alice.String(), + Denom: denom, + }) + requireT.NoError(err) + requireT.Equal(aliceBaseAmount, balanceRes.Balance.Int64(), "Alice should have 1,000,000 tokens") + requireT.Equal(frozenAmount, balanceRes.Frozen.Int64(), "Alice should have 600,000 frozen tokens") + spendableAmount := aliceBaseAmount - frozenAmount // 400,000 + spendable := balanceRes.Balance.Sub(balanceRes.Frozen).Sub(balanceRes.LockedInDEX) + requireT.Equal(spendableAmount, spendable.Int64(), "Alice should have 400,000 spendable tokens") + + t.Logf("=== Initial state: balance=%d, frozen=%d, dexLocked=0, spendable=%d ===", + aliceBaseAmount, frozenAmount, spendableAmount) + + // ======================================================================== + // PHASE 1: Verify that frozen tokens cannot be DEX-locked via orders + // + // If the fix were missing: we could place multiple sell orders that together + // lock more than spendable (400k + 400k + 200k = 1M, but only 400k spendable). + // Each order would pass because validation checked (balance - dexLocked) + // without subtracting frozen. The fix ensures only spendable tokens can be locked. + // ======================================================================== + + // Step 1: Place first sell order for 400,000 tokens (the unfrozen portion) — must succeed + order1Amount := int64(400_000) // 40 * 10,000 + placeOrder1 := &dextypes.MsgPlaceOrder{ + Sender: alice.String(), + Type: dextypes.ORDER_TYPE_LIMIT, + ID: "order1", + BaseDenom: denom, + QuoteDenom: quoteDenom, + Price: lo.ToPtr(dextypes.MustNewPriceFromString("1")), + Quantity: sdkmath.NewInt(order1Amount), + Side: dextypes.SIDE_SELL, + TimeInForce: dextypes.TIME_IN_FORCE_GTC, + } + _, err = client.BroadcastTx( + ctx, + chain.ClientContext.WithFromAddress(alice), + chain.TxFactoryAuto(), + placeOrder1, + ) + requireT.NoError(err, "First order of 400,000 should pass (400,000 unfrozen available)") + + balanceRes, err = assetFTClient.Balance(ctx, &assetfttypes.QueryBalanceRequest{ + Account: alice.String(), + Denom: denom, + }) + requireT.NoError(err) + requireT.Equal(order1Amount, balanceRes.LockedInDEX.Int64(), "400,000 tokens should be DEX-locked") + t.Logf("Step 1: Placed order for %d tokens. dexLocked=%d", order1Amount, order1Amount) + + // Step 2: Place second sell order for 400,000 MORE tokens — must FAIL + // Available spendable = 1,000,000 - 400,000 (dexLocked) - 600,000 (frozen) = 0. + // If the fix were missing, (balance - dexLocked) = 600,000 >= 400,000 would incorrectly pass. + order2Amount := int64(400_000) // 40 * 10,000 + placeOrder2 := &dextypes.MsgPlaceOrder{ + Sender: alice.String(), + Type: dextypes.ORDER_TYPE_LIMIT, + ID: "order2", + BaseDenom: denom, + QuoteDenom: quoteDenom, + Price: lo.ToPtr(dextypes.MustNewPriceFromString("1")), + Quantity: sdkmath.NewInt(order2Amount), + Side: dextypes.SIDE_SELL, + TimeInForce: dextypes.TIME_IN_FORCE_GTC, + } + _, err = client.BroadcastTx( + ctx, + chain.ClientContext.WithFromAddress(alice), + chain.TxFactoryAuto(), + placeOrder2, + ) + requireT.Error(err, "Second order of 400,000 must fail: it would lock frozen tokens; available spendable is 0") + t.Logf("Step 2: Second order of %d correctly rejected — frozen tokens cannot be DEX-locked", order2Amount) + + // Verify state unchanged: still 400,000 DEX-locked, 600,000 frozen + balanceRes, err = assetFTClient.Balance(ctx, &assetfttypes.QueryBalanceRequest{ + Account: alice.String(), + Denom: denom, + }) + requireT.NoError(err) + requireT.Equal(order1Amount, balanceRes.LockedInDEX.Int64(), "DEX-locked should remain 400,000") + requireT.Equal(frozenAmount, balanceRes.Frozen.Int64(), "Frozen should remain 600,000") +} diff --git a/x/asset/ft/keeper/keeper_dex.go b/x/asset/ft/keeper/keeper_dex.go index 7d202d65..aaee944e 100644 --- a/x/asset/ft/keeper/keeper_dex.go +++ b/x/asset/ft/keeper/keeper_dex.go @@ -66,6 +66,26 @@ func (k Keeper) DEXExecuteActions(ctx sdk.Context, actions types.DEXActions) err } for _, send := range actions.Send { + // Validate frozen balances before sending to prevent transferring frozen tokens + def, err := k.getDefinitionOrNil(ctx, send.Coin.Denom) + if err != nil { + return err + } + if def != nil && def.IsFeatureEnabled(types.Feature_freezing) && !def.HasAdminPrivileges(send.FromAddress) { + balance := k.bankKeeper.GetBalance(ctx, send.FromAddress, send.Coin.Denom) + dexLocked := k.GetDEXLockedBalance(ctx, send.FromAddress, send.Coin.Denom).Amount + bankLocked := k.bankKeeper.LockedCoins(ctx, send.FromAddress).AmountOf(send.Coin.Denom) + frozenBalance, err := k.GetFrozenBalance(ctx, send.FromAddress, send.Coin.Denom) + if err != nil { + return err + } + available := balance.Amount.Sub(dexLocked).Sub(bankLocked).Sub(frozenBalance.Amount) + if available.LT(send.Coin.Amount) { + return sdkerrors.Wrapf(cosmoserrors.ErrInsufficientFunds, + "cannot send %s: insufficient spendable balance (frozen tokens cannot be transferred)", + send.Coin.String()) + } + } k.logger(ctx).Debug( "DEX sending coin", "from", send.FromAddress.String(), @@ -666,6 +686,24 @@ func (k Keeper) validateCoinIsNotLockedByDEXAndBank( coin.String(), availableAmt.String(), coin.Denom) } + // validate that we don't use frozen coins (if freezing feature is enabled) + def, err := k.getDefinitionOrNil(ctx, coin.Denom) + if err != nil { + return err + } + if def != nil && def.IsFeatureEnabled(types.Feature_freezing) && !def.HasAdminPrivileges(addr) { + frozenBalance, err := k.GetFrozenBalance(ctx, addr, coin.Denom) + if err != nil { + return err + } + frozenAmt := frozenBalance.Amount + notFrozenAmt := availableAmt.Sub(frozenAmt) + if notFrozenAmt.LT(coin.Amount) { + return sdkerrors.Wrapf(cosmoserrors.ErrInsufficientFunds, "%s is not available, available %s%s", + coin.String(), notFrozenAmt.String(), coin.Denom) + } + } + return nil } diff --git a/x/asset/ft/keeper/keeper_dex_test.go b/x/asset/ft/keeper/keeper_dex_test.go index dda8b7ab..202a990b 100644 --- a/x/asset/ft/keeper/keeper_dex_test.go +++ b/x/asset/ft/keeper/keeper_dex_test.go @@ -1151,3 +1151,116 @@ func TestKeeper_DEXExtensions(t *testing.T) { ) // since DEXCheckOrderAmounts is failed, won't check DEXInvokeAssetExtension } + +// TestKeeper_FrozenTokenEscapeViaDEX verifies that frozen tokens cannot be +// DEX-locked or transferred via DEX settlement. +// +// If the fix were missing: DEXIncreaseLocked would allow locking frozen tokens +// (it only checked balance - dexLocked - bankLocked), and DEXExecuteActions would +// transfer them via raw bank keeper, bypassing FT hooks. This test asserts the +// fix: the second lock attempt (which would lock frozen tokens) must fail. +func TestKeeper_FrozenTokenEscapeViaDEX(t *testing.T) { + requireT := require.New(t) + + testApp := simapp.New() + ctx := testApp.NewContextLegacy(false, tmproto.Header{ + Time: time.Now(), + AppHash: []byte("some-hash"), + }) + + ftKeeper := testApp.AssetFTKeeper + bankKeeper := testApp.BankKeeper + + // ======================================================================== + // SETUP: Create token with Feature_freezing, fund Alice, freeze tokens + // ======================================================================== + + issuer := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + alice := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + bob := sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()) + + // Issue a token with the freezing feature enabled (the "base" token Alice sells) + settings := types.IssueSettings{ + Issuer: issuer, + Symbol: "VULN", + Subunit: "vuln", + Precision: 6, + InitialAmount: sdkmath.NewInt(1_000_000), + Features: []types.Feature{types.Feature_freezing}, + } + denom, err := ftKeeper.Issue(ctx, settings) + requireT.NoError(err) + + // Issue a second token (the "quote" token Bob uses to buy) + quoteSettings := types.IssueSettings{ + Issuer: issuer, + Symbol: "QUOTE", + Subunit: "quote", + Precision: 6, + InitialAmount: sdkmath.NewInt(1_000_000), + Features: []types.Feature{}, + } + quoteDenom, err := ftKeeper.Issue(ctx, quoteSettings) + requireT.NoError(err) + + // Fund Alice with 100 base tokens + aliceCoins := sdk.NewInt64Coin(denom, 100) + requireT.NoError(bankKeeper.SendCoins(ctx, issuer, alice, sdk.NewCoins(aliceCoins))) + + // Fund Bob with 100 quote tokens (for buying) + bobCoins := sdk.NewInt64Coin(quoteDenom, 100) + requireT.NoError(bankKeeper.SendCoins(ctx, issuer, bob, sdk.NewCoins(bobCoins))) + + // Admin freezes 60 of Alice's base tokens + freezeAmount := sdk.NewInt64Coin(denom, 60) + requireT.NoError(ftKeeper.Freeze(ctx, issuer, alice, freezeAmount)) + + // Verify initial state + balance := bankKeeper.GetBalance(ctx, alice, denom) + requireT.Equal(int64(100), balance.Amount.Int64(), "Alice should have 100 tokens") + + frozenBalance, err := ftKeeper.GetFrozenBalance(ctx, alice, denom) + requireT.NoError(err) + requireT.Equal(int64(60), frozenBalance.Amount.Int64(), "Alice should have 60 frozen tokens") + + spendable, err := ftKeeper.GetSpendableBalance(ctx, alice, denom) + requireT.NoError(err) + requireT.Equal(int64(40), spendable.Amount.Int64(), "Alice should have 40 spendable tokens") + + t.Log("=== Initial state: balance=100, frozen=60, dexLocked=0, spendable=40 ===") + + // ======================================================================== + // PHASE 1: Verify that frozen tokens cannot be DEX-locked + // + // If the vulnerability existed: DEXIncreaseLocked would only check + // (balance - dexLocked - bankLocked >= amount) without considering frozen. + // That would allow locking 40 more tokens (overlapping with frozen), then + // 20 more, until all 60 frozen tokens were DEX-locked and transferable. + // The fix: validateCoinIsNotLockedByDEXAndBank now subtracts frozen from + // available balance, so only truly spendable tokens can be DEX-locked. + // ======================================================================== + + // Step 1: Lock 40 tokens (the unfrozen portion) — must succeed + lockAmount1 := sdk.NewInt64Coin(denom, 40) + err = ftKeeper.DEXIncreaseLocked(ctx, alice, lockAmount1) + requireT.NoError(err, "First lock of 40 should pass (40 unfrozen available)") + + dexLocked := ftKeeper.GetDEXLockedBalance(ctx, alice, denom) + requireT.Equal(int64(40), dexLocked.Amount.Int64(), "40 tokens should be DEX-locked") + t.Log("Step 1: DEXIncreaseLocked(40) passed. dexLocked=40") + + // Step 2: Lock 40 MORE tokens — must FAIL (would lock frozen tokens) + // Available spendable = 100 - 40 (dexLocked) - 60 (frozen) = 0. + // If the fix were missing, (balance - dexLocked) = 60 >= 40 would incorrectly pass. + lockAmount2 := sdk.NewInt64Coin(denom, 40) + err = ftKeeper.DEXIncreaseLocked(ctx, alice, lockAmount2) + requireT.Error(err, "Second lock of 40 must fail: it would lock frozen tokens; available spendable is 0") + t.Log("Step 2: DEXIncreaseLocked(40) correctly rejected — frozen tokens cannot be DEX-locked") + + // Verify state unchanged: still 40 DEX-locked, 60 frozen + dexLocked = ftKeeper.GetDEXLockedBalance(ctx, alice, denom) + requireT.Equal(int64(40), dexLocked.Amount.Int64(), "DEX-locked should remain 40") + frozenBalance, err = ftKeeper.GetFrozenBalance(ctx, alice, denom) + requireT.NoError(err) + requireT.Equal(int64(60), frozenBalance.Amount.Int64(), "Frozen should remain 60") +}