lnwire: preserve unknown odd zero-length final hop TLVs
What the developer wrote
Authored by bitromortac
lnwire: preserve unknown odd zero-length final hop TLVs
When decoding an onion message payload, the loop that forwards
unrecognized final hop TLVs to higher layers skipped any entry with a
zero-length value. DecodeWithParsedTypesP2P marks a recognized type with
a nil map entry but records the raw bytes for an unknown type, and an
unknown odd TLV with an empty value is valid. Keying the skip off a
length check therefore dropped such a TLV instead of passing it through.
Test the recognized-type skip against a nil entry so a valid unknown odd
zero-length TLV is preserved.
What changed, and why it matters
This commit fixes a small but real bug in how LND decodes onion-routed messages (used in Lightning Network offers and blinded paths). Unknown extra data fields with a valid but empty value were accidentally dropped because the code used a length check to decide what was 'known.' After the fix, it uses a nil check instead, so legitimate empty unknown fields are preserved. The bug could cause a node to silently ignore data another node expected it to forward or process, potentially breaking protocol features that rely on those fields.
Treat as a normal bug-fix patch. Reviewers should confirm the nil-vs-empty distinction is consistent across other TLV decode paths in lnwire, and consider whether any other components use len(tlvBytes) == 0 to detect recognized types. No immediate security response appears necessary, but operators should include this fix in their next maintenance update to avoid interoperability issues with future BOLT12 features.
Security signals we found
Loss of protocol data: valid unknown odd zero-length TLVs were dropped during decode
TLV parsing logic conflated 'recognized type' (nil map entry) with 'zero-length value' (empty byte slice)
Fix changes skip condition from length check to nil check
New regression test covers the zero-length unknown odd TLV case
No vendor security framing, CVE, or researcher attribution present in commit
Evidence from the diff
In lnwire/onion_msg_payload.go, OnionMessagePayload.Decode builds a list of FinalHopTLVs by walking the parsed TLV map and skipping recognized types. DecodeWithParsedTypesP2P stores nil for recognized types and the raw bytes for unknown types. The old code skipped entries where len(tlvBytes) == 0, which incorrectly also skipped unknown odd TLVs with zero-length values (a valid TLV encoding). The fix changes the condition to tlvBytes == nil and updates the related comment. A unit test is added to ensure an unknown odd zero-length TLV round-trips correctly.
Changed components
lnwire/onion_msg_payload.golnwire/onion_msg_payload_test.goOnionMessagePayload.DecodeFinalHopTLV handling for BOLT12/blinded-path messagesInspect captured patch +30 / −4
diff --git a/lnwire/onion_msg_payload.go b/lnwire/onion_msg_payload.go
index f91c650..7b384d9 100644
--- a/lnwire/onion_msg_payload.go
+++ b/lnwire/onion_msg_payload.go
@@ -179,9 +179,13 @@ func (o *OnionMessagePayload) Decode(r io.Reader) (map[tlv.Type][]byte, error) {
continue
}
- // Skip any tlvs that have been recognized in our decoding (a
- // zero entry means that we recognized the entry).
- if len(tlvBytes) == 0 {
+ // Skip any tlvs that have been recognized in our decoding.
+ // DecodeWithParsedTypesP2P stores a nil entry for known types
+ // that it decoded into a dedicated field above, and the raw
+ // bytes for unknown types. A nil check (rather than a length
+ // check) is required so that a valid unknown odd tlv with a
+ // zero-length value is not mistaken for a recognized type.
+ if tlvBytes == nil {
continue
}
@@ -199,7 +203,7 @@ func (o *OnionMessagePayload) Decode(r io.Reader) (map[tlv.Type][]byte, error) {
// If we read out an invoice, invoice error or invoice request tlv
// sub-namespace, add it to our set of final payloads. This value won't
// have been added in the loop above, because we recognized the TLV so
- // len(tlvMap[invoiceType].tlvBytes) will be zero (thus, skipped above).
+ // tlvMap[invoiceType].tlvBytes will be nil (thus, skipped above).
if _, ok := tlvMap[InvoiceNamespaceType]; ok {
o.FinalHopTLVs = append(
o.FinalHopTLVs, invoicePayload,
diff --git a/lnwire/onion_msg_payload_test.go b/lnwire/onion_msg_payload_test.go
index 6871f99..4cc464d 100644
--- a/lnwire/onion_msg_payload_test.go
+++ b/lnwire/onion_msg_payload_test.go
@@ -294,6 +294,28 @@ func TestOnionMessagePayloadRoundTrip(t *testing.T) {
decoded.FinalHopTLVs[0].Value,
)
})
+
+ t.Run("odd unknown zero-length final hop TLV", func(t *testing.T) {
+ t.Parallel()
+
+ // A valid unknown odd tlv with a zero-length value must be
+ // preserved rather than mistaken for a recognized type, which
+ // is why decode keys off a nil entry instead of an empty one.
+ original := &OnionMessagePayload{
+ FinalHopTLVs: []*FinalHopTLV{
+ {
+ TLVType: 65,
+ Value: []byte{},
+ },
+ },
+ }
+
+ decoded := encodeAndDecode(t, original)
+
+ require.Len(t, decoded.FinalHopTLVs, 1)
+ require.Equal(t, tlv.Type(65), decoded.FinalHopTLVs[0].TLVType)
+ require.Empty(t, decoded.FinalHopTLVs[0].Value)
+ })
}
// TestFinalHopTLVValidate tests that FinalHopTLV.Validate correctly rejects
Why this scored 49/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.