multi: allow invalid tlv data when decoding payment fail info
What changed, and why it matters
This change makes LND more forgiving when reading old payment failure records that contain slightly corrupted extra data fields. Previously, such records could cause the database read to fail entirely; now the code logs a warning and continues, leaving the failure message empty but preserving the rest of the record. It is a robustness fix rather than an active vulnerability patch.
Treat as a defensive hardening/robustness improvement. Review whether silently discarding the failure message affects downstream payment routing or debugging, and ensure the warning log is monitored. No urgent security patch is indicated by the diff alone.
Security signals we found
Change from hard deserialization failure to warning-and-continue for malformed stored TLV data
New sentinel error used for precise error matching
Unit test demonstrates that truncated TLV no longer aborts HTLCFailInfo deserialization
Failure message is dropped when extra TLV parsing fails
Evidence from the diff
The commit introduces a sentinel error, ErrParsingExtraTLVBytes, returned when ChannelUpdate1’s extra opaque bytes cannot be parsed as a valid TLV stream. DecodeFailureMessage now wraps the underlying error. deserializeHTLCFailInfo catches that specific error and continues deserialization, setting the failure message to nil, instead of aborting. A unit test constructs a TemporaryChannelFailure with a truncated inbound-fee TLV and verifies graceful handling.
Changed components
lnwire.ChannelUpdate1.Decodelnwire.DecodeFailureMessagepayments/db.kv_store.deserializeHTLCFailInfoInspect captured patch +139 / −3
diff --git a/lnwire/channel_update.go b/lnwire/channel_update.go
index 558d81a..2dc4a11 100644
--- a/lnwire/channel_update.go
+++ b/lnwire/channel_update.go
@@ -169,7 +169,7 @@ func (a *ChannelUpdate1) Decode(r io.Reader, _ uint32) error {
var inboundFee = a.InboundFee.Zero()
typeMap, err := tlvRecords.ExtractRecords(&inboundFee)
if err != nil {
- return err
+ return fmt.Errorf("%w: %w", ErrParsingExtraTLVBytes, err)
}
val, ok := typeMap[a.InboundFee.TlvType()]
diff --git a/lnwire/error.go b/lnwire/error.go
index fcc22ef..3824119 100644
--- a/lnwire/error.go
+++ b/lnwire/error.go
@@ -6,6 +6,13 @@ import (
"io"
)
+var (
+ // ErrParsingExtraTLVBytes is returned when we attempt to parse
+ // extra opaque bytes as a TLV stream, but the parsing fails due to
+ // and invalid TLV stream.
+ ErrParsingExtraTLVBytes = fmt.Errorf("error parsing extra TLV bytes")
+)
+
// FundingError represents a set of errors that can be encountered and sent
// during the funding workflow.
type FundingError uint8
diff --git a/lnwire/onion_error.go b/lnwire/onion_error.go
index 2cbf548..9cc115f 100644
--- a/lnwire/onion_error.go
+++ b/lnwire/onion_error.go
@@ -1361,7 +1361,7 @@ func DecodeFailureMessage(r io.Reader, pver uint32) (FailureMessage, error) {
case Serializable:
if err := f.Decode(r, pver); err != nil {
return nil, fmt.Errorf("unable to decode error "+
- "update (type=%T): %v", failure, err)
+ "update (type=%T): %w", failure, err)
}
}
diff --git a/payments/db/kv_store.go b/payments/db/kv_store.go
index e5915a0..62f0b83 100644
--- a/payments/db/kv_store.go
+++ b/payments/db/kv_store.go
@@ -2099,9 +2099,19 @@ func deserializeHTLCFailInfo(r io.Reader) (*HTLCFailInfo, error) {
f.Message, err = lnwire.DecodeFailureMessage(
bytes.NewReader(failureBytes), 0,
)
- if err != nil {
+ if err != nil &&
+ !errors.Is(err, lnwire.ErrParsingExtraTLVBytes) {
+
return nil, err
}
+
+ // In case we have an invalid TLV stream regarding the extra
+ // tlv data we still continue with the decoding of the
+ // HTLCFailInfo.
+ if errors.Is(err, lnwire.ErrParsingExtraTLVBytes) {
+ log.Warnf("Failed to decode extra TLV bytes for "+
+ "failure message: %v", err)
+ }
}
var reason byte
diff --git a/payments/db/kv_store_test.go b/payments/db/kv_store_test.go
index 73df8ea..2c28951 100644
--- a/payments/db/kv_store_test.go
+++ b/payments/db/kv_store_test.go
@@ -2,11 +2,15 @@ package paymentsdb
import (
"bytes"
+ "encoding/binary"
+ "io"
"math"
"reflect"
"testing"
"time"
+ "github.com/btcsuite/btcd/btcec/v2/ecdsa"
+ "github.com/btcsuite/btcd/wire"
"github.com/btcsuite/btcwallet/walletdb"
"github.com/lightningnetwork/lnd/kvdb"
"github.com/lightningnetwork/lnd/lntypes"
@@ -1070,3 +1074,118 @@ func TestLazySessionKeyDeserialize(t *testing.T) {
sessionKey := attempt.SessionKey()
require.Equal(t, priv, sessionKey)
}
+
+// TestDeserializeHTLCFailInfoInvalidTLV tests that deserializeHTLCFailInfo
+// handles invalid extra tlv data gracefully by not failing.
+func TestDeserializeHTLCFailInfoInvalidTLV(t *testing.T) {
+ // Create a channel update with valid data first, then encode it.
+ testSig := &ecdsa.Signature{}
+ sig, _ := lnwire.NewSigFromSignature(testSig)
+ chanUpdate := &lnwire.ChannelUpdate1{
+ Signature: sig,
+ ShortChannelID: lnwire.NewShortChanIDFromInt(1),
+ Timestamp: 1,
+ MessageFlags: 0,
+ ChannelFlags: 1,
+ ExtraOpaqueData: make([]byte, 0),
+ }
+
+ var chanUpdateBuf bytes.Buffer
+ err := chanUpdate.Encode(&chanUpdateBuf, 0)
+ require.NoError(t, err)
+
+ // Append invalid inbound fee TLV record to the encoded channel update.
+ // The inbound fee TLV has type 55555 and should have 8 bytes of data
+ // (2 uint32 values: BaseFee and FeeRate). We create an invalid one by
+ // using the correct type but with incomplete data (only 6 bytes
+ // instead of 8).
+ var invalidInboundFeeTLV bytes.Buffer
+
+ // Write type 55555 as varint: 0xfd + 2 bytes (canonical encoding)
+ err = invalidInboundFeeTLV.WriteByte(0xfd)
+ require.NoError(t, err)
+
+ var typeBytes [2]byte
+ binary.BigEndian.PutUint16(typeBytes[:], 55555)
+ _, err = invalidInboundFeeTLV.Write(typeBytes[:])
+ require.NoError(t, err)
+
+ // Write length as 8 (single byte since 8 < 0xfd, no varint needed)
+ err = invalidInboundFeeTLV.WriteByte(8)
+ require.NoError(t, err)
+
+ // Write only 6 bytes of value data (incomplete, should be 8 bytes)
+ var valueBytes [6]byte
+ binary.BigEndian.PutUint32(valueBytes[0:4], 1)
+ binary.BigEndian.PutUint16(valueBytes[4:6], 2)
+ _, err = invalidInboundFeeTLV.Write(valueBytes[:])
+ require.NoError(t, err)
+
+ _, err = chanUpdateBuf.Write(invalidInboundFeeTLV.Bytes())
+ require.NoError(t, err)
+
+ // Manually create a TemporaryChannelFailure failure message with the
+ // corrupted channel update bytes.
+ var failureMsgBuf bytes.Buffer
+
+ // Write the failure code.
+ err = lnwire.WriteUint16(
+ &failureMsgBuf, uint16(lnwire.CodeTemporaryChannelFailure),
+ )
+ require.NoError(t, err)
+
+ // Write the length of the channel update (including invalid TLV).
+ err = lnwire.WriteUint16(&failureMsgBuf, uint16(chanUpdateBuf.Len()))
+ require.NoError(t, err)
+
+ // Write the channel update bytes with invalid TLV appended.
+ _, err = failureMsgBuf.Write(chanUpdateBuf.Bytes())
+ require.NoError(t, err)
+
+ _, err = lnwire.DecodeFailureMessage(&failureMsgBuf, 0)
+ require.ErrorIs(t, err, lnwire.ErrParsingExtraTLVBytes)
+ require.ErrorIs(t, err, io.ErrUnexpectedEOF)
+
+ // Create an HTLCFailInfo and serialize it with the corrupted failure
+ // message.
+ failInfo := &HTLCFailInfo{
+ FailTime: time.Now(),
+ Reason: HTLCFailMessage,
+ FailureSourceIndex: 2,
+ }
+
+ var buf bytes.Buffer
+
+ // Manually serialize the HTLCFailInfo with the corrupted failure bytes.
+ err = serializeTime(&buf, failInfo.FailTime)
+ require.NoError(t, err)
+
+ // Write the failure message bytes.
+ err = wire.WriteVarBytes(&buf, 0, failureMsgBuf.Bytes())
+ require.NoError(t, err)
+
+ // Write reason and failure source index.
+ err = WriteElements(
+ &buf, byte(failInfo.Reason), failInfo.FailureSourceIndex,
+ )
+ require.NoError(t, err)
+
+ // Now deserialize the HTLCFailInfo - this should NOT fail despite the
+ // invalid TLV data.
+ deserializedFailInfo, err := deserializeHTLCFailInfo(&buf)
+ require.NoError(t, err, "deserializeHTLCFailInfo should not fail "+
+ "with invalid TLV data")
+ require.NotNil(t, deserializedFailInfo)
+
+ // Verify the basic fields are correctly deserialized.
+ require.Equal(t, failInfo.Reason, deserializedFailInfo.Reason)
+ require.Equal(t, failInfo.FailureSourceIndex,
+ deserializedFailInfo.FailureSourceIndex)
+
+ // Verify the failure message is nil because the decoding failed
+ // due to invalid TLV data. The important part is that the
+ // HTLCFailInfo deserialization still succeeded despite the invalid
+ // TLV data in the failure message.
+ require.Nil(t, deserializedFailInfo.Message,
+ "Message should be nil when TLV parsing fails")
+}
Why this scored 47/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.