From b14d54476eff149e900dca89ce8942240f534b05 Mon Sep 17 00:00:00 2001 From: Enrico Rubboli Date: Thu, 24 Sep 2026 11:52:32 +0200 Subject: [PATCH 1/3] fix(wasm): stop double-freeing guest-owned Vec params (heap corruption) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wasm-bindgen reference-types ABI transfers ownership of Vec / Vec> argument arrays to the guest: during the call the guest converts every element, recycles the externref table slots and frees the array buffer itself. The wrapper's deferred freeStringArray / freeUint8ArrayArray then operated on freed memory — reading garbage table indices, re-deallocing guest slots (poisoning the slot free-list) and double-freeing the array buffer — corrupting the dlmalloc heap. The next malloc-heavy export then trapped OOB (encode_partially_signed_transaction, encode_witness_no_signature, ..., wherever the damaged chunk was hit first). Affected exports: - estimate_transaction_size (EstimateTransactionSize) — hit by the fee fixed-point loop up to 3x before the first PST encode, which is why the conclude+create flow was the live incident - encode_signed_transaction_intent (EncodeSignedTransactionIntent) - verify_transaction_intent (VerifyTransactionIntent) Fix: mirror wasm-bindgen's own JS glue for owned-Vec params. The array writers now return an externRefArray that tracks what the wrapper still owns; post-call release() frees only the Go refs entries (plus the wrapper-owned element backing buffers for byte arrays, which the guest only copies from, never frees) and never touches the guest-owned array buffer or re-reads indices from it. discard() handles error paths where the export was never invoked and the wrapper still owns everything. Regression tests (wasm/ownership_regression_test.go) reproduce the corruption deterministically on the old code — estimate x3 then encode canary traps OOB, intent-encode canary yields mangled heap reads — and pass after the fix. Full suite green, including repeat runs. --- wasm/client.go | 180 ++++++++++++------------------ wasm/intent.go | 19 +++- wasm/ownership_regression_test.go | 145 ++++++++++++++++++++++++ wasm/transactions.go | 11 +- 4 files changed, 240 insertions(+), 115 deletions(-) create mode 100644 wasm/ownership_regression_test.go diff --git a/wasm/client.go b/wasm/client.go index 1478808..ee61fb6 100644 --- a/wasm/client.go +++ b/wasm/client.go @@ -361,161 +361,139 @@ func (c *Client) readAmount(wasmPtr uint32) (Amount, error) { // wasm-bindgen's passArrayJsValueToWasm0 pattern. const externrefTableIndex = 1 +// externRefArray is the Go-side bookkeeping for an array of externref values +// passed to a WASM export (wasm-bindgen reference-types passArrayJsValueToWasm0 +// pattern). +// +// OWNERSHIP: the array buffer (Ptr) is transferred to the guest when the +// export is invoked. During the call the guest converts every element, +// recycles the externref table slots, and frees the buffer itself — exactly +// like wasm-bindgen's own JS glue for owned Vec params, which never frees the +// buffer post-call. The wrapper must therefore NEVER free Ptr or re-read the +// table indices from it after the export ran (doing so is a double free that +// corrupts the dlmalloc heap). Only the Go refs registry entries — and, for +// byte-array elements, the wrapper-allocated backing buffers — remain owned +// by the wrapper and are released by release. +type externRefArray struct { + ptr uint32 // array buffer; owned by the guest once the export is called + count uint32 + keys []uintptr // Go refs registry entries for the elements + elems []uint8ArrayRef // wrapper-owned element backing buffers (byte arrays only) +} + +// release frees everything the wrapper still owns after an export consumed +// the array. It must be called after the export returned (success or domain +// error). It never touches the array buffer or the externref table: the guest +// consumed and freed those during the call. +func (a *externRefArray) release(c *Client) { + for i, key := range a.keys { + if i < len(a.elems) { + c.freeWASM(a.elems[i].ptr, a.elems[i].len) + } + refs.free(key) + } +} + +// discard frees everything including the array buffer. Only valid on error +// paths where the export was never invoked, so the wrapper still owns it. +func (a *externRefArray) discard(c *Client) { + a.release(c) + c.freeWASM(a.ptr, a.count*4) +} + // writeStringArray writes a []string as an array of WASM externref table indices in // WASM linear memory, matching the passArrayJsValueToWasm0 pattern. -// Returns (ptr, count). Call freeStringArray to release. -func (c *Client) writeStringArray(strs []string) (ptr, count uint32, err error) { +// The returned array must be released after the consuming export runs +// (release); discard it (discard) only if the export is never called. +func (c *Client) writeStringArray(strs []string) (externRefArray, error) { + var arr externRefArray if len(strs) == 0 { - return 0, 0, nil + return arr, nil } n := uint32(len(strs)) mallocResult, callErr := c.mod.ExportedFunction("__wbindgen_malloc").Call(c.ctx, uint64(n*4), 4) if callErr != nil || len(mallocResult) == 0 { - return 0, 0, fmt.Errorf("mintlayer: malloc for string array: %w", callErr) + return arr, fmt.Errorf("mintlayer: malloc for string array: %w", callErr) } - arrPtr := uint32(mallocResult[0]) + arr.ptr = uint32(mallocResult[0]) + arr.count = n allocFn := c.mod.ExportedFunction("__externref_table_alloc") if allocFn == nil { - c.freeWASM(arrPtr, n*4) - return 0, 0, fmt.Errorf("mintlayer: __externref_table_alloc not found") + arr.discard(c) + return externRefArray{}, fmt.Errorf("mintlayer: __externref_table_alloc not found") } - tableIndices := make([]uint32, 0, n) - for _, s := range strs { + for i, s := range strs { idxResult, err2 := allocFn.Call(c.ctx) if err2 != nil || len(idxResult) == 0 { - // Cleanup already-allocated slots - for _, idx := range tableIndices { - key := getWASMTableRef(c.mod, externrefTableIndex, idx) - refs.free(key) - } - c.freeWASM(arrPtr, n*4) - return 0, 0, fmt.Errorf("mintlayer: __externref_table_alloc: %w", err2) + arr.discard(c) // export never called: wrapper still owns everything + return externRefArray{}, fmt.Errorf("mintlayer: __externref_table_alloc: %w", err2) } tableIdx := uint32(idxResult[0]) - tableIndices = append(tableIndices, tableIdx) // Store string in Go refs and wire it into the WASM table. key := refs.alloc(s) + arr.keys = append(arr.keys, key) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) var buf [4]byte binary.LittleEndian.PutUint32(buf[:], tableIdx) - c.mod.Memory().Write(arrPtr+uint32(len(tableIndices)-1)*4, buf[:]) + c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) } - return arrPtr, n, nil + return arr, nil } // writeUint8ArrayArray writes a [][]byte as an array of WASM externref table indices. // Each byte slice is first copied into WASM heap and wrapped as a uint8ArrayRef. -func (c *Client) writeUint8ArrayArray(slices [][]byte) (ptr, count uint32, err error) { +// The element backing buffers stay wrapper-owned (the guest only copies from +// them); see externRefArray for the ownership rules. +func (c *Client) writeUint8ArrayArray(slices [][]byte) (externRefArray, error) { + var arr externRefArray if len(slices) == 0 { - return 0, 0, nil + return arr, nil } n := uint32(len(slices)) mallocResult, callErr := c.mod.ExportedFunction("__wbindgen_malloc").Call(c.ctx, uint64(n*4), 4) if callErr != nil || len(mallocResult) == 0 { - return 0, 0, fmt.Errorf("mintlayer: malloc for byte-array array: %w", callErr) + return arr, fmt.Errorf("mintlayer: malloc for byte-array array: %w", callErr) } - arrPtr := uint32(mallocResult[0]) + arr.ptr = uint32(mallocResult[0]) + arr.count = n allocFn := c.mod.ExportedFunction("__externref_table_alloc") if allocFn == nil { - c.freeWASM(arrPtr, n*4) - return 0, 0, fmt.Errorf("mintlayer: __externref_table_alloc not found") - } - - type allocation struct { - tableIdx uint32 - wasmPtr uint32 - wasmLen uint32 + arr.discard(c) + return externRefArray{}, fmt.Errorf("mintlayer: __externref_table_alloc not found") } - allocs := make([]allocation, 0, n) for _, b := range slices { wasmPtr, wasmLen, err2 := c.writeBytes(b) if err2 != nil { - for _, a := range allocs { - key := getWASMTableRef(c.mod, externrefTableIndex, a.tableIdx) - refs.free(key) - c.freeWASM(a.wasmPtr, a.wasmLen) - } - c.freeWASM(arrPtr, n*4) - return 0, 0, fmt.Errorf("mintlayer: write bytes for array: %w", err2) + arr.discard(c) // export never called: wrapper still owns everything + return externRefArray{}, fmt.Errorf("mintlayer: write bytes for array: %w", err2) } idxResult, err2 := allocFn.Call(c.ctx) if err2 != nil || len(idxResult) == 0 { c.freeWASM(wasmPtr, wasmLen) - for _, a := range allocs { - key := getWASMTableRef(c.mod, externrefTableIndex, a.tableIdx) - refs.free(key) - c.freeWASM(a.wasmPtr, a.wasmLen) - } - c.freeWASM(arrPtr, n*4) - return 0, 0, fmt.Errorf("mintlayer: __externref_table_alloc: %w", err2) + arr.discard(c) + return externRefArray{}, fmt.Errorf("mintlayer: __externref_table_alloc: %w", err2) } tableIdx := uint32(idxResult[0]) key := refs.alloc(uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) + arr.keys = append(arr.keys, key) + arr.elems = append(arr.elems, uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) - allocs = append(allocs, allocation{tableIdx, wasmPtr, wasmLen}) var buf [4]byte binary.LittleEndian.PutUint32(buf[:], tableIdx) - c.mod.Memory().Write(arrPtr+uint32(len(allocs)-1)*4, buf[:]) + c.mod.Memory().Write(arr.ptr+uint32(len(arr.keys)-1)*4, buf[:]) } - return arrPtr, n, nil -} - -// freeStringArray releases resources from writeStringArray. -func (c *Client) freeStringArray(arrPtr, count uint32) { - if arrPtr == 0 { - return - } - deallocFn := c.mod.ExportedFunction("__externref_table_dealloc") - for i := uint32(0); i < count; i++ { - data, ok := c.mod.Memory().Read(arrPtr+i*4, 4) - if !ok { - continue - } - tableIdx := binary.LittleEndian.Uint32(data) - key := getWASMTableRef(c.mod, externrefTableIndex, tableIdx) - refs.free(key) - if deallocFn != nil { - deallocFn.Call(c.ctx, uint64(tableIdx)) //nolint:errcheck - } - } - c.freeWASM(arrPtr, count*4) -} - -// freeUint8ArrayArray releases resources from writeUint8ArrayArray. -func (c *Client) freeUint8ArrayArray(arrPtr, count uint32) { - if arrPtr == 0 { - return - } - deallocFn := c.mod.ExportedFunction("__externref_table_dealloc") - for i := uint32(0); i < count; i++ { - data, ok := c.mod.Memory().Read(arrPtr+i*4, 4) - if !ok { - continue - } - tableIdx := binary.LittleEndian.Uint32(data) - key := getWASMTableRef(c.mod, externrefTableIndex, tableIdx) - if v, ok2 := refs.get(key); ok2 { - if arr, ok3 := v.(uint8ArrayRef); ok3 { - c.freeWASM(arr.ptr, arr.len) - } - } - refs.free(key) - if deallocFn != nil { - deallocFn.Call(c.ctx, uint64(tableIdx)) //nolint:errcheck - } - } - c.freeWASM(arrPtr, count*4) + return arr, nil } // allocExternRef stores val in the Go refs registry and returns the key. @@ -545,16 +523,6 @@ func setWASMTableRef(mod api.Module, tableIdx, refIdx uint32, val uintptr) { refs[refIdx] = val } -// getWASMTableRef reads a value from the WASM table. -func getWASMTableRef(mod api.Module, tableIdx, refIdx uint32) uintptr { - refsSlice, ok := wasmTableRefsSlice(mod, tableIdx) - if !ok || int(refIdx) >= refsSlice.Len() { - return 0 - } - refs := unsafe.Slice((*uintptr)(refsSlice.UnsafePointer()), refsSlice.Len()) - return refs[refIdx] -} - // wasmTableRefsSlice returns the reflect.Value of the References slice for a table. func wasmTableRefsSlice(mod api.Module, tableIdx uint32) (reflect.Value, bool) { mv := reflect.ValueOf(mod) diff --git a/wasm/intent.go b/wasm/intent.go index 14b59b7..e01635a 100644 --- a/wasm/intent.go +++ b/wasm/intent.go @@ -32,13 +32,18 @@ func (c *Client) EncodeSignedTransactionIntent(signedMessage []byte, signatures if err != nil { return nil, err } - sigsPtr, sigsLen, err := c.writeUint8ArrayArray(signatures) + // The signatures array buffer is owned by the guest once the export is + // invoked (it converts the elements and frees the buffer during the call); + // only the Go-side refs entries and the wrapper-owned element backing + // buffers are released afterwards. Freeing the array buffer here would be + // a double free corrupting the WASM heap. + sigs, err := c.writeUint8ArrayArray(signatures) if err != nil { return nil, err } - defer c.freeUint8ArrayArray(sigsPtr, sigsLen) + defer sigs.release(c) return c.callReturnBytes("encode_signed_transaction_intent", - uint64(msgPtr), uint64(msgLen), uint64(sigsPtr), uint64(sigsLen)) + uint64(msgPtr), uint64(msgLen), uint64(sigs.ptr), uint64(sigs.count)) } // VerifyTransactionIntent verifies a signed transaction intent. @@ -54,14 +59,16 @@ func (c *Client) VerifyTransactionIntent(expectedSignedMessage, encodedSignedInt if err != nil { return err } - destsPtr, destsLen, err := c.writeStringArray(inputDestinations) + // The destinations array buffer is owned by the guest once the export is + // invoked; only the Go-side refs entries are released afterwards. + dests, err := c.writeStringArray(inputDestinations) if err != nil { return err } - defer c.freeStringArray(destsPtr, destsLen) + defer dests.release(c) return c.callVoidFallible("verify_transaction_intent", uint64(msgPtr), uint64(msgLen), uint64(intentPtr), uint64(intentLen), - uint64(destsPtr), uint64(destsLen), + uint64(dests.ptr), uint64(dests.count), uint64(network)) } diff --git a/wasm/ownership_regression_test.go b/wasm/ownership_regression_test.go new file mode 100644 index 0000000..1ba6dc0 --- /dev/null +++ b/wasm/ownership_regression_test.go @@ -0,0 +1,145 @@ +// Copyright (c) 2026 Mintlayer Institutional FZCO +// Contact: hello@mintlayer.org +// +// Use of this source code is governed by an MIT license +// that can be found in the LICENSE file. + +package mintlayer_test + +// Regression tests for the externref-array ownership bug (dlmalloc heap +// corruption). +// +// The WASM module's Vec/Vec> ABI (wasm-bindgen reference +// types) TRANSFERS OWNERSHIP of the argument array to the guest: during the +// call the guest converts every element, recycles the externref table slots, +// and frees the array buffer itself. The wrapper must therefore never free +// the array buffer (or re-read indices from it) after the call — doing so is +// a double free that corrupts the dlmalloc heap, and the next malloc-heavy +// export traps out-of-bounds. +// +// These tests are canaries: they perform a vec-parameter call followed by a +// malloc-heavy export and assert the latter still works. + +import ( + "testing" + + mintlayer "github.com/mintlayer/go-sdk/wasm" +) + +// deriveTestAddress returns a valid bech32m address on the given network. +func deriveTestAddress(t *testing.T, c *mintlayer.Client, index uint32, network mintlayer.Network) string { + t.Helper() + mnemonic := "abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon abandon about" + accountKey, err := c.MakeDefaultAccountPrivkey(mnemonic, network) + if err != nil { + t.Fatalf("MakeDefaultAccountPrivkey: %v", err) + } + recvKey, err := c.MakeReceivingAddress(accountKey, index) + if err != nil { + t.Fatalf("MakeReceivingAddress: %v", err) + } + pubKey, err := c.PublicKeyFromPrivateKey(recvKey) + if err != nil { + t.Fatalf("PublicKeyFromPrivateKey: %v", err) + } + addr, err := c.PubkeyToPubkeyHashAddress(pubKey, network) + if err != nil { + t.Fatalf("PubkeyToPubkeyHashAddress: %v", err) + } + return addr +} + +// heapCanary runs malloc-heavy exports and fails if the heap has been +// corrupted by an earlier double free. +func heapCanary(t *testing.T, c *mintlayer.Client) { + t.Helper() + if _, err := c.EncodeWitnessNoSignature(); err != nil { + t.Fatalf("heap corruption canary: EncodeWitnessNoSignature trapped: %v", err) + } + input := buildFakeInput(t, c) + output, err := c.EncodeOutputTransfer(mintlayer.NewAmount("100000000000"), deriveTestAddress(t, c, 99, mintlayer.Mainnet), mintlayer.Mainnet) + if err != nil { + t.Fatalf("heap corruption canary: EncodeOutputTransfer: %v", err) + } + if _, err := c.EncodeTransaction(input, output, 0); err != nil { + t.Fatalf("heap corruption canary: EncodeTransaction trapped: %v", err) + } +} + +// TestEstimateTransactionSizeThenEncodeCanary mirrors the fee fixed-point +// loop: several EstimateTransactionSize calls with owned string-vec args, +// followed by encode-heavy exports. Regression for the double free of the +// estimate_transaction_size destinations array. +func TestEstimateTransactionSizeThenEncodeCanary(t *testing.T) { + c := newClient(t) + + input := buildFakeInput(t, c) + output, err := c.EncodeOutputTransfer(mintlayer.NewAmount("100000000000"), deriveTestAddress(t, c, 1, mintlayer.Mainnet), mintlayer.Mainnet) + if err != nil { + t.Fatalf("EncodeOutputTransfer: %v", err) + } + dests := []string{ + deriveTestAddress(t, c, 2, mintlayer.Mainnet), + deriveTestAddress(t, c, 3, mintlayer.Mainnet), + } + + // 3× like the fee fixed-point loop before the first PST encode. + for i := 0; i < 3; i++ { + size, err := c.EstimateTransactionSize(input, dests, output, mintlayer.Mainnet) + if err != nil { + t.Fatalf("EstimateTransactionSize (round %d): %v", i, err) + } + if size == 0 { + t.Fatalf("EstimateTransactionSize (round %d): returned 0", i) + } + // A canary after every estimate: pre-fix, the deferred free of the + // destinations array corrupts the heap and this traps immediately. + heapCanary(t, c) + } +} + +// TestEncodeSignedTransactionIntentCanary covers the Vec> path +// (writeUint8ArrayArray): the guest frees the signatures array buffer during +// encode_signed_transaction_intent, so the wrapper must not free it again. +func TestEncodeSignedTransactionIntentCanary(t *testing.T) { + c := newClient(t) + + message, err := c.MakeTransactionIntentMessageToSign("intent-1", "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20") + if err != nil { + t.Fatalf("MakeTransactionIntentMessageToSign: %v", err) + } + privKey, err := c.MakePrivateKey() + if err != nil { + t.Fatalf("MakePrivateKey: %v", err) + } + sig, err := c.SignChallenge(privKey, message) + if err != nil { + t.Fatalf("SignChallenge: %v", err) + } + sigs := [][]byte{sig, sig, sig} + + if _, err := c.EncodeSignedTransactionIntent(message, sigs); err != nil { + t.Logf("EncodeSignedTransactionIntent: %v (domain error is fine; the call still consumes the vec)", err) + } + heapCanary(t, c) +} + +// TestVerifyTransactionIntentCanary covers the second Vec path +// (VerifyTransactionIntent input destinations). +func TestVerifyTransactionIntentCanary(t *testing.T) { + c := newClient(t) + + message, err := c.MakeTransactionIntentMessageToSign("intent-1", "0102030405060708090a0b0c0d0e0f101112131415161718191a1b1c1d1e1f20") + if err != nil { + t.Fatalf("MakeTransactionIntentMessageToSign: %v", err) + } + dests := []string{ + deriveTestAddress(t, c, 4, mintlayer.Mainnet), + deriveTestAddress(t, c, 5, mintlayer.Mainnet), + } + + if err := c.VerifyTransactionIntent(message, []byte{0x01, 0x02, 0x03}, dests, mintlayer.Mainnet); err != nil { + t.Logf("VerifyTransactionIntent: %v (domain error is fine; the call still consumes the vec)", err) + } + heapCanary(t, c) +} diff --git a/wasm/transactions.go b/wasm/transactions.go index 27e13e5..3fb0b6e 100644 --- a/wasm/transactions.go +++ b/wasm/transactions.go @@ -52,18 +52,23 @@ func (c *Client) EstimateTransactionSize(inputs []byte, inputUtxosDests []string if err != nil { return 0, err } - destsPtr, destsLen, err := c.writeStringArray(inputUtxosDests) + // The destinations array buffer is owned by the guest once the export is + // invoked (it converts the elements and frees the buffer during the call); + // only the Go-side refs entries are released afterwards. Freeing the buffer + // here would be a double free corrupting the WASM heap. + dests, err := c.writeStringArray(inputUtxosDests) if err != nil { return 0, err } - defer c.freeStringArray(destsPtr, destsLen) + defer dests.release(c) outPtr, outLen, err := c.writeBytes(outputs) if err != nil { + dests.discard(c) // export never called: wrapper still owns the buffer return 0, err } return c.callReturnU32("estimate_transaction_size", uint64(inPtr), uint64(inLen), - uint64(destsPtr), uint64(destsLen), + uint64(dests.ptr), uint64(dests.count), uint64(outPtr), uint64(outLen), uint64(network)) } From 8fca27858082142b9014fa33f66def339f131ab3 Mon Sep 17 00:00:00 2001 From: Enrico Rubboli Date: Thu, 24 Sep 2026 12:12:28 +0200 Subject: [PATCH 2/3] fix(wasm): harden externRefArray cleanup (OCR review findings) Address all four findings from the OpenCodeReview run 35985298671: - critical (bug): release()/discard() are now idempotent (they zero the record after cleanup), so a discard followed by the deferred release can never free refs/element buffers twice. - medium (maintainability): EstimateTransactionSize no longer registers the deferred release before the outputs writeBytes error path; discard now runs alone on that path (no overlap with a pending release). - medium (bug): discard() now recycles the externref table slots via __externref_table_dealloc (single i32 slot index, verified against the module export signature) using table indices captured at write time, instead of permanently leaking slots on pre-invocation error paths. - low (maintainability): writeUint8ArrayArray writes element indices at offset i (loop index), consistent with writeStringArray, instead of len(arr.keys)-1. --- wasm/client.go | 37 ++++++++++++++++++++++++++++++------- wasm/transactions.go | 4 ++-- 2 files changed, 32 insertions(+), 9 deletions(-) diff --git a/wasm/client.go b/wasm/client.go index ee61fb6..45afa3d 100644 --- a/wasm/client.go +++ b/wasm/client.go @@ -378,27 +378,48 @@ type externRefArray struct { ptr uint32 // array buffer; owned by the guest once the export is called count uint32 keys []uintptr // Go refs registry entries for the elements - elems []uint8ArrayRef // wrapper-owned element backing buffers (byte arrays only) + idxs []uint32 // externref table slots (recyclable until the export consumes them) + elems []uint8ArrayRef // wrapper-owned element backing buffers (byte arrays only) } // release frees everything the wrapper still owns after an export consumed // the array. It must be called after the export returned (success or domain // error). It never touches the array buffer or the externref table: the guest -// consumed and freed those during the call. +// consumed and freed those during the call. Idempotent. func (a *externRefArray) release(c *Client) { + if a.ptr == 0 && len(a.keys) == 0 { + return // empty or already released + } for i, key := range a.keys { if i < len(a.elems) { c.freeWASM(a.elems[i].ptr, a.elems[i].len) } refs.free(key) } + a.keys = nil + a.elems = nil + a.idxs = nil + a.ptr = 0 + a.count = 0 } -// discard frees everything including the array buffer. Only valid on error -// paths where the export was never invoked, so the wrapper still owns it. +// discard frees everything, including the array buffer and the externref +// table slots. Only valid on error paths where the export was never invoked: +// the guest has not recycled the slots or freed the buffer yet, so the +// wrapper still owns all of it. Idempotent. func (a *externRefArray) discard(c *Client) { + if a.ptr == 0 && len(a.keys) == 0 { + return // empty or already discarded + } + ptr, count := a.ptr, a.count + deallocFn := c.mod.ExportedFunction("__externref_table_dealloc") + for _, idx := range a.idxs { + if deallocFn != nil { + deallocFn.Call(c.ctx, uint64(idx)) //nolint:errcheck + } + } a.release(c) - c.freeWASM(a.ptr, a.count*4) + c.freeWASM(ptr, count*4) } // writeStringArray writes a []string as an array of WASM externref table indices in @@ -436,6 +457,7 @@ func (c *Client) writeStringArray(strs []string) (externRefArray, error) { // Store string in Go refs and wire it into the WASM table. key := refs.alloc(s) arr.keys = append(arr.keys, key) + arr.idxs = append(arr.idxs, tableIdx) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) var buf [4]byte @@ -469,7 +491,7 @@ func (c *Client) writeUint8ArrayArray(slices [][]byte) (externRefArray, error) { return externRefArray{}, fmt.Errorf("mintlayer: __externref_table_alloc not found") } - for _, b := range slices { + for i, b := range slices { wasmPtr, wasmLen, err2 := c.writeBytes(b) if err2 != nil { arr.discard(c) // export never called: wrapper still owns everything @@ -486,12 +508,13 @@ func (c *Client) writeUint8ArrayArray(slices [][]byte) (externRefArray, error) { key := refs.alloc(uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) arr.keys = append(arr.keys, key) + arr.idxs = append(arr.idxs, tableIdx) arr.elems = append(arr.elems, uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) var buf [4]byte binary.LittleEndian.PutUint32(buf[:], tableIdx) - c.mod.Memory().Write(arr.ptr+uint32(len(arr.keys)-1)*4, buf[:]) + c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) } return arr, nil } diff --git a/wasm/transactions.go b/wasm/transactions.go index 3fb0b6e..269d3d0 100644 --- a/wasm/transactions.go +++ b/wasm/transactions.go @@ -60,12 +60,12 @@ func (c *Client) EstimateTransactionSize(inputs []byte, inputUtxosDests []string if err != nil { return 0, err } - defer dests.release(c) outPtr, outLen, err := c.writeBytes(outputs) if err != nil { - dests.discard(c) // export never called: wrapper still owns the buffer + dests.discard(c) // export never called: wrapper still owns everything return 0, err } + defer dests.release(c) return c.callReturnU32("estimate_transaction_size", uint64(inPtr), uint64(inLen), uint64(dests.ptr), uint64(dests.count), From d730860abd0a1391846385b4db3479700dfffecd Mon Sep 17 00:00:00 2001 From: Enrico Rubboli Date: Thu, 24 Sep 2026 12:21:03 +0200 Subject: [PATCH 3/3] fix(wasm): address OCR round-2 findings - free wrapper-owned pre-export buffers on every pre-invocation error path: inputs in EstimateTransactionSize (2 paths), message/intent buffers in both intent functions (writeBytes inputs leak on array writer failure) - check Memory().Write result when writing table indices into the array buffer in both writers; on failure discard the whole array and error out instead of returning success with an uninitialized slot - fold keys/idxs/elems parallel slices into a single per-element record (externRefElement), removing positional coupling between the slices - writeBytes: free the malloc'd buffer if the memory write fails instead of returning 0 (caller could never free it) - distinguish pre-invocation call failures (unknown export name, arity mismatch) via errExportNotInvoked: externRefArray.finish discards the array when the export never ran, and releases otherwise (leak-safe on traps, where the guest may have partially consumed the buffer) - drop duplicated externRefArray doc comment left above externRefElement --- wasm/client.go | 92 ++++++++++++++++++++++++++++++-------------- wasm/intent.go | 20 ++++++++-- wasm/transactions.go | 11 +++++- 3 files changed, 89 insertions(+), 34 deletions(-) diff --git a/wasm/client.go b/wasm/client.go index 45afa3d..e0ae1ed 100644 --- a/wasm/client.go +++ b/wasm/client.go @@ -11,6 +11,7 @@ import ( _ "embed" "encoding/binary" "encoding/json" + "errors" "fmt" "reflect" "sync" @@ -61,6 +62,14 @@ func (c *Client) Close() error { // ── low-level call helpers ──────────────────────────────────────────────────── +// errExportNotInvoked marks call errors that happened before the WASM export +// started executing (unknown name, arity mismatch). The guest cannot have +// taken ownership of any wrapper-allocated arguments, so callers may still +// discard (fully free) externRefArrays passed to the failed call. Any other +// error (domain error, trap) means the export ran — possibly partially — so +// arrays must be released (leak-safe) instead of discarded. +var errExportNotInvoked = errors.New("export was not invoked") + // call executes a named WASM export function with the given parameters and // returns the raw uint64 results. func (c *Client) call(fn string, params ...uint64) ([]uint64, error) { @@ -73,7 +82,11 @@ func (c *Client) call(fn string, params ...uint64) ([]uint64, error) { ctx := ctxWithCall(c.ctx) f := c.mod.ExportedFunction(fn) if f == nil { - return nil, fmt.Errorf("mintlayer: function %q not found", fn) + return nil, fmt.Errorf("mintlayer: function %q not found: %w", fn, errExportNotInvoked) + } + if len(params) != len(f.Definition().ParamTypes()) { + return nil, fmt.Errorf("mintlayer: function %q: got %d params, want %d: %w", + fn, len(params), len(f.Definition().ParamTypes()), errExportNotInvoked) } res, err := f.Call(ctx, params...) @@ -284,6 +297,7 @@ func (c *Client) writeBytes(data []byte) (ptr, length uint32, err error) { } ptr = uint32(result[0]) if !c.mod.Memory().Write(ptr, data) { + c.freeWASM(ptr, uint32(len(data))) // nothing handed out, wrapper still owns it return 0, 0, fmt.Errorf("mintlayer: memory write failed") } return ptr, uint32(len(data)), nil @@ -361,25 +375,32 @@ func (c *Client) readAmount(wasmPtr uint32) (Amount, error) { // wasm-bindgen's passArrayJsValueToWasm0 pattern. const externrefTableIndex = 1 +// externRefElement is one element of an externRefArray: a Go refs registry +// entry, the externref table slot it occupies, and — for byte-array elements +// only — the wrapper-owned backing buffer the guest copies from. +type externRefElement struct { + key uintptr + idx uint32 + buf uint8ArrayRef // zero value for string elements +} + // externRefArray is the Go-side bookkeeping for an array of externref values // passed to a WASM export (wasm-bindgen reference-types passArrayJsValueToWasm0 // pattern). // -// OWNERSHIP: the array buffer (Ptr) is transferred to the guest when the +// OWNERSHIP: the array buffer (ptr) is transferred to the guest when the // export is invoked. During the call the guest converts every element, // recycles the externref table slots, and frees the buffer itself — exactly // like wasm-bindgen's own JS glue for owned Vec params, which never frees the -// buffer post-call. The wrapper must therefore NEVER free Ptr or re-read the -// table indices from it after the export ran (doing so is a double free that -// corrupts the dlmalloc heap). Only the Go refs registry entries — and, for -// byte-array elements, the wrapper-allocated backing buffers — remain owned -// by the wrapper and are released by release. +// buffer post-call. The wrapper must therefore NEVER free ptr or touch the +// table slots after the export ran (doing so is a double free that corrupts +// the dlmalloc heap). The per-element Go refs entries — and, for byte-array +// elements, the wrapper-allocated backing buffers — remain owned by the +// wrapper and are released by release. type externRefArray struct { ptr uint32 // array buffer; owned by the guest once the export is called count uint32 - keys []uintptr // Go refs registry entries for the elements - idxs []uint32 // externref table slots (recyclable until the export consumes them) - elems []uint8ArrayRef // wrapper-owned element backing buffers (byte arrays only) + elems []externRefElement } // release frees everything the wrapper still owns after an export consumed @@ -387,18 +408,14 @@ type externRefArray struct { // error). It never touches the array buffer or the externref table: the guest // consumed and freed those during the call. Idempotent. func (a *externRefArray) release(c *Client) { - if a.ptr == 0 && len(a.keys) == 0 { + if a.ptr == 0 && len(a.elems) == 0 { return // empty or already released } - for i, key := range a.keys { - if i < len(a.elems) { - c.freeWASM(a.elems[i].ptr, a.elems[i].len) - } - refs.free(key) + for _, el := range a.elems { + c.freeWASM(el.buf.ptr, el.buf.len) + refs.free(el.key) } - a.keys = nil a.elems = nil - a.idxs = nil a.ptr = 0 a.count = 0 } @@ -408,20 +425,36 @@ func (a *externRefArray) release(c *Client) { // the guest has not recycled the slots or freed the buffer yet, so the // wrapper still owns all of it. Idempotent. func (a *externRefArray) discard(c *Client) { - if a.ptr == 0 && len(a.keys) == 0 { + if a.ptr == 0 && len(a.elems) == 0 { return // empty or already discarded } ptr, count := a.ptr, a.count deallocFn := c.mod.ExportedFunction("__externref_table_dealloc") - for _, idx := range a.idxs { + for _, el := range a.elems { if deallocFn != nil { - deallocFn.Call(c.ctx, uint64(idx)) //nolint:errcheck + deallocFn.Call(c.ctx, uint64(el.idx)) //nolint:errcheck } } a.release(c) c.freeWASM(ptr, count*4) } +// finish cleans up arr after an export attempt returned invokeErr. If the +// export was never invoked (unknown name, arity mismatch) the wrapper still +// owns everything: the array is discarded and finish reports true so the +// caller can free any sibling wrapper-owned buffers handed to the same +// export. Otherwise (success, domain error, or trap — the guest ran, possibly +// partially) the guest owns the buffer and slots and only the wrapper-owned +// records are released. +func (a *externRefArray) finish(c *Client, invokeErr error) bool { + if invokeErr != nil && errors.Is(invokeErr, errExportNotInvoked) { + a.discard(c) + return true + } + a.release(c) + return false +} + // writeStringArray writes a []string as an array of WASM externref table indices in // WASM linear memory, matching the passArrayJsValueToWasm0 pattern. // The returned array must be released after the consuming export runs @@ -456,13 +489,15 @@ func (c *Client) writeStringArray(strs []string) (externRefArray, error) { // Store string in Go refs and wire it into the WASM table. key := refs.alloc(s) - arr.keys = append(arr.keys, key) - arr.idxs = append(arr.idxs, tableIdx) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) + arr.elems = append(arr.elems, externRefElement{key: key, idx: tableIdx}) var buf [4]byte binary.LittleEndian.PutUint32(buf[:], tableIdx) - c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) + if !c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) { + arr.discard(c) // export never called: wrapper still owns everything + return externRefArray{}, fmt.Errorf("mintlayer: write table index %d for string array: out of bounds", i) + } } return arr, nil } @@ -507,14 +542,15 @@ func (c *Client) writeUint8ArrayArray(slices [][]byte) (externRefArray, error) { tableIdx := uint32(idxResult[0]) key := refs.alloc(uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) - arr.keys = append(arr.keys, key) - arr.idxs = append(arr.idxs, tableIdx) - arr.elems = append(arr.elems, uint8ArrayRef{ptr: wasmPtr, len: wasmLen}) setWASMTableRef(c.mod, externrefTableIndex, tableIdx, key) + arr.elems = append(arr.elems, externRefElement{key: key, idx: tableIdx, buf: uint8ArrayRef{ptr: wasmPtr, len: wasmLen}}) var buf [4]byte binary.LittleEndian.PutUint32(buf[:], tableIdx) - c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) + if !c.mod.Memory().Write(arr.ptr+uint32(i)*4, buf[:]) { + arr.discard(c) // export never called: wrapper still owns everything + return externRefArray{}, fmt.Errorf("mintlayer: write table index %d for byte-array array: out of bounds", i) + } } return arr, nil } diff --git a/wasm/intent.go b/wasm/intent.go index e01635a..04ee7bf 100644 --- a/wasm/intent.go +++ b/wasm/intent.go @@ -39,11 +39,15 @@ func (c *Client) EncodeSignedTransactionIntent(signedMessage []byte, signatures // a double free corrupting the WASM heap. sigs, err := c.writeUint8ArrayArray(signatures) if err != nil { + c.freeWASM(msgPtr, msgLen) // export never called: wrapper still owns the message return nil, err } - defer sigs.release(c) - return c.callReturnBytes("encode_signed_transaction_intent", + res, err := c.callReturnBytes("encode_signed_transaction_intent", uint64(msgPtr), uint64(msgLen), uint64(sigs.ptr), uint64(sigs.count)) + if sigs.finish(c, err) { + c.freeWASM(msgPtr, msgLen) // export was never invoked + } + return res, err } // VerifyTransactionIntent verifies a signed transaction intent. @@ -57,18 +61,26 @@ func (c *Client) VerifyTransactionIntent(expectedSignedMessage, encodedSignedInt } intentPtr, intentLen, err := c.writeBytes(encodedSignedIntent) if err != nil { + c.freeWASM(msgPtr, msgLen) // export never called: wrapper still owns the message return err } // The destinations array buffer is owned by the guest once the export is // invoked; only the Go-side refs entries are released afterwards. dests, err := c.writeStringArray(inputDestinations) if err != nil { + c.freeWASM(msgPtr, msgLen) // export never called: wrapper still owns the message + c.freeWASM(intentPtr, intentLen) // and the encoded intent return err } - defer dests.release(c) - return c.callVoidFallible("verify_transaction_intent", + err = c.callVoidFallible("verify_transaction_intent", uint64(msgPtr), uint64(msgLen), uint64(intentPtr), uint64(intentLen), uint64(dests.ptr), uint64(dests.count), uint64(network)) + if dests.finish(c, err) { + // Export was never invoked: wrapper still owns the byte buffers too. + c.freeWASM(msgPtr, msgLen) + c.freeWASM(intentPtr, intentLen) + } + return err } diff --git a/wasm/transactions.go b/wasm/transactions.go index 269d3d0..10d7f7c 100644 --- a/wasm/transactions.go +++ b/wasm/transactions.go @@ -58,19 +58,26 @@ func (c *Client) EstimateTransactionSize(inputs []byte, inputUtxosDests []string // here would be a double free corrupting the WASM heap. dests, err := c.writeStringArray(inputUtxosDests) if err != nil { + c.freeWASM(inPtr, inLen) // export never called: wrapper still owns inputs return 0, err } outPtr, outLen, err := c.writeBytes(outputs) if err != nil { dests.discard(c) // export never called: wrapper still owns everything + c.freeWASM(inPtr, inLen) return 0, err } - defer dests.release(c) - return c.callReturnU32("estimate_transaction_size", + size, err := c.callReturnU32("estimate_transaction_size", uint64(inPtr), uint64(inLen), uint64(dests.ptr), uint64(dests.count), uint64(outPtr), uint64(outLen), uint64(network)) + if dests.finish(c, err) { + // Export was never invoked: wrapper still owns the byte buffers too. + c.freeWASM(inPtr, inLen) + c.freeWASM(outPtr, outLen) + } + return size, err } // EncodeSignedTransaction combines an unsigned transaction with its witness signatures into