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.

Author: hieblmi

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,
 		)

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.

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},

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) {