From db9bd0662977d68c0f38e3eca75f8e08c41c8ec8 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Wed, 27 May 2026 12:19:34 +0200 Subject: [PATCH] staticaddr: reject malformed MuSig2 signing data Server-supplied nonces and partial signatures are consumed by the static address loop-in and withdrawal MuSig2 signing paths. Reject nil signing info, wrong nonce lengths, and wrong partial signature lengths before registering nonces or combining signatures, so malformed responses cannot be silently zero-padded into signing attempts. Add withdrawal coverage for nil and malformed server signing data. --- staticaddr/loopin/manager.go | 12 ++-- staticaddr/loopin/manager_test.go | 78 ++++++++++++++++++++++ staticaddr/withdraw/manager.go | 21 +++++- staticaddr/withdraw/manager_test.go | 100 ++++++++++++++++++++++++++++ 4 files changed, 205 insertions(+), 6 deletions(-) diff --git a/staticaddr/loopin/manager.go b/staticaddr/loopin/manager.go index a41c7ced..a76cbba3 100644 --- a/staticaddr/loopin/manager.go +++ b/staticaddr/loopin/manager.go @@ -379,12 +379,14 @@ func (m *Manager) handleLoopInSweepReq(ctx context.Context, return err } - var ( - serverNonce [musig2.PubNonceSize]byte - sigHash [32]byte - ) + var sigHash [32]byte + + serverNonce, err := byteSliceTo66ByteSlice(nonce) + if err != nil { + return fmt.Errorf("invalid server nonce for "+ + "deposit %v: %w", depositOutpoint, err) + } - copy(serverNonce[:], nonce) musig2Session, err := staticutil.CreateMusig2Session( ctx, m.cfg.Signer, loopIn.AddressParams, loopIn.Address, ) diff --git a/staticaddr/loopin/manager_test.go b/staticaddr/loopin/manager_test.go index 8d9b11a9..85178a05 100644 --- a/staticaddr/loopin/manager_test.go +++ b/staticaddr/loopin/manager_test.go @@ -1,11 +1,14 @@ package loopin import ( + "bytes" "context" "errors" "testing" + "github.com/btcsuite/btcd/btcec/v2/schnorr/musig2" "github.com/btcsuite/btcd/btcutil" + "github.com/btcsuite/btcd/btcutil/psbt" "github.com/btcsuite/btcd/chaincfg/chainhash" "github.com/btcsuite/btcd/wire" "github.com/lightninglabs/loop" @@ -14,6 +17,7 @@ import ( "github.com/lightninglabs/loop/staticaddr/deposit" "github.com/lightninglabs/loop/staticaddr/script" "github.com/lightninglabs/loop/swap" + "github.com/lightninglabs/loop/swapserverrpc" "github.com/lightningnetwork/lnd/lntypes" "github.com/lightningnetwork/lnd/routing/route" "github.com/lightningnetwork/lnd/zpay32" @@ -220,6 +224,80 @@ func TestInitiateLoopInAllowsReservedAutoloopLabel(t *testing.T) { require.Equal(t, selectedDeposit.Value, quoteGetter.amount) } +// TestHandleLoopInSweepReqRejectsInvalidServerNonce ensures that a malformed +// MuSig2 nonce returned by the server is rejected before it reaches the signer. +func TestHandleLoopInSweepReqRejectsInvalidServerNonce(t *testing.T) { + ctx := t.Context() + + changeAddr := &script.Parameters{ + PkScript: []byte{0xaa, 0xbb}, + } + + const confirmationHeight = 0 + dep := makeDeposit(7, 0, 10_000, confirmationHeight) + depOutpoint := outpointString(dep) + + swapHash := lntypes.Hash{9} + loopIn := &StaticAddressLoopIn{ + SwapHash: swapHash, + DepositOutpoints: []string{depOutpoint}, + SelectedAmount: dep.Value, + } + loopIn.SetState(Succeeded) + + sweepTx := makeSweepTx( + []wire.OutPoint{dep.OutPoint}, + []*wire.TxOut{{ + Value: int64(dep.Value), + PkScript: []byte{0xcc, 0xdd}, + }}, + ) + sweepPacket, err := psbt.NewFromUnsignedTx(sweepTx) + require.NoError(t, err) + + var psbtBuf bytes.Buffer + require.NoError(t, sweepPacket.Serialize(&psbtBuf)) + + mgr := &Manager{ + cfg: &Config{ + AddressManager: &mockAddressManager{ + params: changeAddr, + }, + DepositManager: &mockDepositManager{ + byOutpoint: map[string]*deposit.Deposit{ + depOutpoint: dep, + }, + }, + Store: &mockStore{ + loopIns: map[lntypes.Hash]*StaticAddressLoopIn{ + swapHash: loopIn, + }, + mapIDs: map[lntypes.Hash][]deposit.ID{ + swapHash: {dep.ID}, + }, + }, + }, + } + + req := &swapserverrpc.ServerStaticLoopInSweepNotification{ + SweepTxPsbt: psbtBuf.Bytes(), + SwapHash: swapHash[:], + DepositToNonces: map[string][]byte{ + depOutpoint: make([]byte, musig2.PubNonceSize-1), + }, + PrevoutInfo: []*swapserverrpc.PrevoutInfo{{ + Value: uint64(dep.Value), + PkScript: changeAddr.PkScript, + TxidBytes: dep.Hash[:], + OutputIndex: dep.Index, + }}, + } + + err = mgr.handleLoopInSweepReq(ctx, req) + require.ErrorContains(t, err, "invalid server nonce") + require.ErrorContains(t, err, depOutpoint) +} + // mockDepositManager implements DepositManager for tests. type mockDepositManager struct { // activeDeposits is the set returned by GetActiveDepositsInState. diff --git a/staticaddr/withdraw/manager.go b/staticaddr/withdraw/manager.go index 99fddd26..6cd94149 100644 --- a/staticaddr/withdraw/manager.go +++ b/staticaddr/withdraw/manager.go @@ -793,13 +793,32 @@ func (m *Manager) signMusig2Tx(ctx context.Context, // We'll now add the nonce to our session and sign the tx. for deposit, sigAndNonce := range sigInfo { + if sigAndNonce == nil { + return nil, fmt.Errorf("missing signing info for "+ + "deposit %v", deposit) + } + session, ok := sessions[deposit] if !ok { return nil, errors.New("session not found") } - nonce := [musig2.PubNonceSize]byte{} + if len(sigAndNonce.Nonce) != musig2.PubNonceSize { + return nil, fmt.Errorf("invalid nonce length for "+ + "deposit %v: got %d, want %d", deposit, + len(sigAndNonce.Nonce), musig2.PubNonceSize) + } + + if len(sigAndNonce.Sig) != input.MuSig2PartialSigSize { + return nil, fmt.Errorf("invalid partial signature "+ + "length for deposit %v: got %d, want %d", + deposit, len(sigAndNonce.Sig), + input.MuSig2PartialSigSize) + } + + var nonce [musig2.PubNonceSize]byte copy(nonce[:], sigAndNonce.Nonce) + haveAllNonces, err := signer.MuSig2RegisterNonces( ctx, session.SessionID, [][musig2.PubNonceSize]byte{nonce}, diff --git a/staticaddr/withdraw/manager_test.go b/staticaddr/withdraw/manager_test.go index 4ffd4e1e..6c883a7c 100644 --- a/staticaddr/withdraw/manager_test.go +++ b/staticaddr/withdraw/manager_test.go @@ -4,6 +4,7 @@ import ( "context" "testing" + "github.com/btcsuite/btcd/btcec/v2/schnorr/musig2" "github.com/btcsuite/btcd/btcutil" "github.com/btcsuite/btcd/chaincfg" "github.com/btcsuite/btcd/chaincfg/chainhash" @@ -377,6 +378,105 @@ func TestSignMusig2Tx_MissingOutpointInDepositMap(t *testing.T) { require.ErrorContains(t, err, "tx outpoint not in deposit index map") } +// TestSignMusig2Tx_InvalidServerSigningInfo tests that malformed server +// signing data is rejected before it is passed to the signer. +func TestSignMusig2Tx_InvalidServerSigningInfo(t *testing.T) { + t.Parallel() + + tx := wire.NewMsgTx(2) + outpoint := wire.OutPoint{ + Hash: [32]byte{1}, + Index: 0, + } + tx.AddTxIn(&wire.TxIn{ + PreviousOutPoint: outpoint, + }) + + pkScript := []byte{ + 0x51, 0x20, // OP_1 OP_PUSHBYTES_32 + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + } + tx.AddTxOut(&wire.TxOut{ + Value: 10000, + PkScript: pkScript, + }) + + depositKey := outpoint.String() + sessions := map[string]*input.MuSig2SessionInfo{ + depositKey: { + SessionID: [32]byte{1}, + }, + } + depositsToIdx := map[string]int{ + depositKey: 0, + } + prevOutFetcher := txscript.NewMultiPrevOutFetcher( + map[wire.OutPoint]*wire.TxOut{ + outpoint: { + Value: 5000, + PkScript: pkScript, + }, + }, + ) + + validNonce := make([]byte, musig2.PubNonceSize) + validSig := make([]byte, input.MuSig2PartialSigSize) + + tests := []struct { + name string + signingInfo *swapserverrpc.ServerPsbtWithdrawSigningInfo + errContains string + }{ + { + name: "nil signing info", + signingInfo: nil, + errContains: "missing signing info", + }, + { + name: "invalid nonce length", + signingInfo: &swapserverrpc.ServerPsbtWithdrawSigningInfo{ + Nonce: validNonce[:musig2.PubNonceSize-1], + Sig: validSig, + }, + errContains: "invalid nonce length", + }, + { + name: "invalid partial signature length", + signingInfo: &swapserverrpc.ServerPsbtWithdrawSigningInfo{ + Nonce: validNonce, + Sig: validSig[:input.MuSig2PartialSigSize-1], + }, + errContains: "invalid partial signature length", + }, + } + + lnd := test.NewMockLnd() + m := &Manager{ + cfg: &ManagerConfig{ + Signer: lnd.Signer, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + sigInfo := map[string]*swapserverrpc.ServerPsbtWithdrawSigningInfo{ + depositKey: tc.signingInfo, + } + + _, err := m.signMusig2Tx( + context.Background(), prevOutFetcher, lnd.Signer, + tx.Copy(), sessions, sigInfo, depositsToIdx, + ) + require.ErrorContains(t, err, tc.errContains) + }) + } +} + // TestCalculateWithdrawalTxValues tests various edge cases in withdrawal // transaction value calculations. func TestCalculateWithdrawalTxValues(t *testing.T) {