From b1bcf61d25e51c3b8319f1c991b28f26d8a53aa6 Mon Sep 17 00:00:00 2001 From: DHEBP <152355273+DHEBP@users.noreply.github.com> Date: Wed, 17 Jun 2026 02:38:57 -0400 Subject: [PATCH] fix(wallet): enforce the native-DERO burn guard on the token-send path TransferToken builds its transfer directly rather than through InternalWalletCall, so it did not share the centralized burn guard the other send paths use. This routes it through the same check. Extract the transfer construction into buildTokenTransfer (credits Amount, Burn 0) and run shouldBlockBurn before building the transaction, so a native-DERO (zero-SCID) burn is consistently rejected across every send path, not just the XSWD path. Add TestTransferTokenCannotBurnNativeDero covering the real constructor: a normal token send is non-destructive and a zero-SCID burn is blocked. --- burn_guard_test.go | 43 ++++++++++++++++++++++++++++++++++++++++++ wallet.go | 47 ++++++++++++++++++++++++++++++++++------------ 2 files changed, 78 insertions(+), 12 deletions(-) diff --git a/burn_guard_test.go b/burn_guard_test.go index 73d5811..b5f8a97 100644 --- a/burn_guard_test.go +++ b/burn_guard_test.go @@ -141,6 +141,49 @@ func TestShouldBlockBurn(t *testing.T) { } } +// TestTransferTokenCannotBurnNativeDero reproduces the v1.0.6 incident at the source level: +// native DERO sent through the token path (TransferToken) with the exact incident signature +// -- native SCID, 1,500,000,000 atomic (15000 DERO), the user-selected ring size of 128 -- +// destroyed the coins because TransferToken built Burn:amount on the zero SCID. This test +// exercises the SAME production constructor TransferToken now uses (buildTokenTransfer), so +// it tests real code, not a re-implementation, and proves two things: +// 1. the corrected constructor emits a NON-destructive transfer (Amount-credited, Burn 0), +// so shouldBlockBurn would NOT trip on a normal send; +// 2. the guard wired into TransferToken catches the destructive shape -- had the old +// Burn:amount construction survived, shouldBlockBurn would have blocked it. +// If anyone reverts buildTokenTransfer to Burn:amount, assertion (1) fails loudly. +func TestTransferTokenCannotBurnNativeDero(t *testing.T) { + const nativeSCID = "0000000000000000000000000000000000000000000000000000000000000000" + const incidentAmount = uint64(1500000000) // 15000.00000 DERO, the exact burned amount + const incidentRing = uint64(128) // user-selected ring size in the incident + _ = incidentRing // ring size is irrelevant to burn classification; pinned for provenance + + dest := "dero1qy976ssakhfynpd4lnh39u7gw9spfzr9z55ckfd0yhrhsdr235glgqq28xlvm" + + // (1) The real constructor TransferToken uses, with the incident's native SCID + amount. + transfers := buildTokenTransfer(nativeSCID, dest, incidentAmount) + if _, block := shouldBlockBurn(transfers, false); block { + t.Fatalf("buildTokenTransfer produced a destructive native-DERO transfer for a normal send -- it must credit Amount with Burn 0, not burn") + } + if got := transfers[0].Burn; got != 0 { + t.Fatalf("buildTokenTransfer set Burn=%d on a token send; must be 0 (the v1.0.6 incident was Burn:amount)", got) + } + if got := transfers[0].Amount; got != incidentAmount { + t.Fatalf("buildTokenTransfer set Amount=%d; the recipient must be credited the full %d", got, incidentAmount) + } + + // (2) Counterfactual: the exact v1.0.6 destructive construction (Burn:amount on zero SCID) + // MUST be classified as a block, proving the guard wired into TransferToken covers it. + v106Destructive := []rpc.Transfer{{Destination: dest, Amount: 0, Burn: incidentAmount, SCID: crypto.ZEROHASH}} + burnAmt, block := shouldBlockBurn(v106Destructive, false) + if !block { + t.Fatalf("the v1.0.6 destructive shape (Burn=%d on zero SCID) was NOT blocked -- the TransferToken guard would not have stopped the incident", incidentAmount) + } + if burnAmt != incidentAmount { + t.Errorf("blocked burn amount = %d, want %d", burnAmt, incidentAmount) + } +} + // TestNoBurnOverrideReintroduced is a source-level sentinel that fails the build if anyone // reintroduces a way to bypass the burn prohibition. HOLOGRAM must never burn DERO: there is // no confirmDestroy flag, no override parameter, no approve path for a destructive burn. If a diff --git a/wallet.go b/wallet.go index 9a7376f..bdba1f7 100644 --- a/wallet.go +++ b/wallet.go @@ -1567,6 +1567,24 @@ func shouldBlockBurn(transfers []rpc.Transfer, hasSCCall bool) (uint64, bool) { return detectDestructiveBurn(transfers, hasSCCall) } +// buildTokenTransfer constructs the transfer for a plain wallet-to-wallet token (or native +// DERO) send. The recipient is credited from Amount on the named SCID -- Burn must be 0. +// Burn is value attached to a smart-contract call (the SC then credits it); with no SC on +// the other end, Amount:0/Burn:amount would debit the sender and credit no one, destroying +// the funds. (Engram's transferAsset uses Amount too.) This was the v1.0.6 incident: native +// DERO sent through the token path with Burn:amount on the zero SCID was destroyed. Kept as +// a standalone helper so the exact production construction is unit-testable without a wallet. +func buildTokenTransfer(scid, destination string, amount uint64) []rpc.Transfer { + return []rpc.Transfer{ + { + Destination: destination, + Amount: amount, // token amount the recipient receives (on this SCID) + Burn: 0, + SCID: crypto.HashHexToHash(scid), + }, + } +} + // InternalWalletCall executes a wallet method directly using the embedded wallet func (a *App) InternalWalletCall(method string, params map[string]interface{}, password string) map[string]interface{} { walletManager.Lock() @@ -3451,18 +3469,23 @@ func (a *App) TransferToken(scid, destination string, amount uint64, password st a.logToConsole(fmt.Sprintf("[Transfer] Transferring %d units of token %s to %s", amount, scid[:16]+"...", destination[:16]+"...")) - // Build transfer with asset (token). For a plain wallet-to-wallet token send, - // the recipient is credited from Amount on the named SCID — Burn must be 0. - // Burn is value attached to a smart-contract call (the SC then credits it); - // with no SC on the other end, Amount:0/Burn:amount would debit the sender and - // credit no one, destroying the tokens. Engram's transferAsset uses Amount too. - transfers := []rpc.Transfer{ - { - Destination: destination, - Amount: amount, // token amount the recipient receives (on this SCID) - Burn: 0, - SCID: crypto.HashHexToHash(scid), - }, + // Credit the recipient from Amount on the named SCID; never Burn (see helper for why). + transfers := buildTokenTransfer(scid, destination, amount) + + // Centralized burn prohibition. TransferToken builds transfers directly and does NOT go + // through InternalWalletCall, so the XSWD-path guard never runs here. Enforce the same + // policy at this chokepoint: a zero-SCID (native DERO) burn with no contract destroys + // coins and is never legitimate. buildTokenTransfer already uses Amount (Burn:0) + // for a normal send; this guard makes the function structurally incapable of emitting a + // destructive burn no matter who calls it (TransferToken is an exported binding) or how + // the construction is later edited. + if burnAmt, block := shouldBlockBurn(transfers, false); block { + a.logToConsole(fmt.Sprintf("[Transfer] BLOCKED native-DERO burn: %s DERO with no contract attached", formatDEROAmount(burnAmt))) + return map[string]interface{}{ + "success": false, + "error": fmt.Sprintf("HOLOGRAM does not allow burning DERO. This request would permanently destroy %s DERO -- a burn with no smart contract attached sends the coins to no one and cannot be undone. If you intend to deliberately burn DERO, use the DERO CLI wallet.", formatDEROAmount(burnAmt)), + "technicalError": fmt.Sprintf("rejected native-DERO burn of %d atomic units (zero SCID, no SC call) in TransferToken; HOLOGRAM prohibits burns", burnAmt), + } } if ringsize < 2 {