← Watch feed
zpay32: reject duplicate payment hash fields
Public commit record
98/100 · Strong
What the developer wrote
Authored by Olaoluwa Osuntokun
zpay32: reject duplicate payment hash fields
In this commit, we reject BOLT 11 invoices that contain more than one
payment hash (`p`) field. `zpay32.Decode` previously kept the first
supported payment hash and ignored later fields, so the interpreted
payment hash depended on field order.
The decoder now tracks whether a `p` field has appeared separately from
whether its contents parsed successfully, and any later `p` field returns
`ErrDuplicatePaymentHash`. This covers identical hashes, distinct hashes,
and a malformed first field followed by a valid one. The change is
deliberately stricter than the current BOLT 11 text, which tells a reader
to use the first `p` field; it is motivated by lightning/bolts#1357, and
will be backported to the v0.20.5 and v0.21.4 releases.
We also replace the stale comment above parse32Bytes, which quoted the
old BOLT 11 rule that a reader must skip over 32-byte fields with an
unsupported length. The current reader requirements say the opposite: a
reader must fail the payment if a fixed-length field (p, h, s, n) does
not have the correct length. The new comment states that reporting the
field as absent is a known divergence, and explains why the end result
for the payment hash is the same either way.
In the tests, we cover identical and distinct duplicate hashes, a
malformed first field followed by a valid one, and the signed vectors. A
valid variant of the flipped vector keeps the wrong-length h and n
fields, so the coverage that such fields are skipped survives. The
duplicate vector in the decode table pins ErrDuplicatePaymentHash
through a new wantErr field rather than asserting any decode error.
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context✓ Explains rationale or failure mode✓ Mentions testing or verification✓ Links an issue, advisory, or supporting reference
What changed, and why it matters
This change tightens how LND reads Lightning invoices. Previously, if an invoice contained more than one payment hash field, LND would silently keep the first valid one and ignore the rest. Now it rejects such invoices outright. This prevents an attacker from potentially tricking a wallet into interpreting a different payment hash than intended, which could lead to payments being sent to the wrong destination or failing in unexpected ways. The fix is being backported to two release branches.
Recommended action
Apply this patch and its backports. Review any integrations that rely on LND accepting invoices with multiple payment hash fields, as they will now fail to decode. Consider monitoring for invoice parsing errors after deployment.
Security signals we found
01
Behavioral change from silent first-field acceptance to explicit rejection of duplicate payment hash fields
02
Prevents invoice parsing from depending on field ordering when multiple payment hashes are present
03
Covers malformed first field + valid second field, closing a potential bypass
04
Backported to v0.20.5 and v0.21.4 release branches, indicating security/robustness relevance
05
Referenced upstream BOLT standard discussion (lightning/bolts#1357)
Evidence from the diff
The patch modifies zpay32.Decode in LND so that any BOLT 11 invoice containing more than one payment hash (‘p’) tagged field returns the new ErrDuplicatePaymentHash. Previously parseTaggedFields kept the first successfully parsed 32-byte payment hash and silently continued over later ones. The decoder now tracks paymentHashSeen independently of parse success, so a malformed first field followed by a valid second field is also rejected. The change is stricter than current BOLT 11 wording and is motivated by lightning/bolts#1357. Tests cover identical, distinct, and malformed-then-valid duplicate hashes, plus signed decode vectors.
Changed components
zpay32/decode.gozpay32/invoice.gozpay32/invoice_internal_test.gozpay32/invoice_test.goInspect captured patch +147 / −10
### docs/release-notes/release-notes-0.20.5.md
@@ -43,6 +43,14 @@
failures](https://github.com/lightningnetwork/lnd/pull/11161), while
preserving the recorded outcome for replayed HTLCs.
+* BOLT 11 invoice decoding [now
+ rejects](https://github.com/lightningnetwork/lnd/pull/11190) invoices that
+ contain more than one payment hash (`p`) field, including duplicate fields
+ with unsupported lengths. This is stricter than the current BOLT 11 text,
+ which tells a reader to use the first `p` field; the change is motivated by
+ [lightning/bolts#1357](https://github.com/lightning/bolts/pull/1357), and
+ is an interop consideration for any wallet emitting such invoices.
+
# New Features
## Functional Enhancements
### docs/release-notes/release-notes-0.21.4.md
@@ -48,6 +48,14 @@
failures](https://github.com/lightningnetwork/lnd/pull/11161), while
preserving the recorded outcome for replayed HTLCs.
+* BOLT 11 invoice decoding [now
+ rejects](https://github.com/lightningnetwork/lnd/pull/11190) invoices that
+ contain more than one payment hash (`p`) field, including duplicate fields
+ with unsupported lengths. This is stricter than the current BOLT 11 text,
+ which tells a reader to use the first `p` field; the change is motivated by
+ [lightning/bolts#1357](https://github.com/lightning/bolts/pull/1357), and
+ is an interop consideration for any wallet emitting such invoices.
+
# New Features
## Functional Enhancements
### zpay32/decode.go
@@ -265,7 +265,11 @@ func parseTimestamp(data []byte) (uint64, error) {
// parseTaggedFields takes the base32 encoded tagged fields of the invoice, and
// fills the Invoice struct accordingly.
func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) error {
- index := 0
+ var (
+ index int
+ paymentHashSeen bool
+ )
+
for len(fields)-index > 0 {
// If there are less than 3 groups to read, there cannot be more
// interesting information, as we need the type (1 group) and
@@ -294,11 +298,10 @@ func parseTaggedFields(invoice *Invoice, fields []byte, net *chaincfg.Params) er
switch typ {
case fieldTypeP:
- if invoice.PaymentHash != nil {
- // We skip the field if we have already seen a
- // supported one.
- continue
+ if paymentHashSeen {
+ return ErrDuplicatePaymentHash
}
+ paymentHashSeen = true
invoice.PaymentHash, err = parse32Bytes(base32Data)
@@ -446,8 +449,14 @@ func parseFieldDataLength(data []byte) (uint16, error) {
func parse32Bytes(data []byte) (*[32]byte, error) {
var paymentHash [32]byte
- // As BOLT-11 states, a reader must skip over the 32-byte fields if
- // it does not have a length of 52, so avoid returning an error.
+ // A field with an unexpected length is reported as absent rather
+ // than as an error, leaving it to the caller to decide whether a
+ // missing field is fatal. Note that BOLT 11 is stricter, and
+ // requires a reader to fail on a fixed-length field (p, h, s, n)
+ // with the wrong length. For the payment hash the end result is the
+ // same: a lone wrong-length field leaves PaymentHash nil and
+ // validateInvoice rejects the invoice, and a wrong-length field
+ // paired with a valid one is rejected as a duplicate.
if len(data) != hashBase32Len {
return nil, nil
}
### zpay32/invoice.go
@@ -103,6 +103,12 @@ var (
// ErrBrokenTaggedField is returned when the last tagged field is
// incorrectly formatted and doesn't have enough bytes to be read.
ErrBrokenTaggedField = errors.New("last tagged field is broken")
+
+ // ErrDuplicatePaymentHash is returned when an invoice contains more
+ // than one payment hash field.
+ ErrDuplicatePaymentHash = errors.New(
+ "invoice contains multiple payment hashes",
+ )
)
// MessageSigner is passed to the Encode method to provide a signature
### zpay32/invoice_internal_test.go
@@ -1,6 +1,7 @@
package zpay32
import (
+ "bytes"
"encoding/binary"
"math"
"reflect"
@@ -781,6 +782,32 @@ func TestParseTaggedFields(t *testing.T) {
netParams := &chaincfg.SimNetParams
+ var malformedThenValid bytes.Buffer
+ require.NoError(t, writeTaggedField(
+ &malformedThenValid, fieldTypeP, []byte{0},
+ ))
+ require.NoError(t, writeBytes32(
+ &malformedThenValid, fieldTypeP, [32]byte{},
+ ))
+
+ var identicalPaymentHashes bytes.Buffer
+ require.NoError(t, writeBytes32(
+ &identicalPaymentHashes, fieldTypeP, testPaymentHash,
+ ))
+ require.NoError(t, writeBytes32(
+ &identicalPaymentHashes, fieldTypeP, testPaymentHash,
+ ))
+
+ var distinctPaymentHashes bytes.Buffer
+ require.NoError(t, writeBytes32(
+ &distinctPaymentHashes, fieldTypeP, testPaymentHash,
+ ))
+ var secondPaymentHash [32]byte
+ copy(secondPaymentHash[:], testDescriptionHash[:])
+ require.NoError(t, writeBytes32(
+ &distinctPaymentHashes, fieldTypeP, secondPaymentHash,
+ ))
+
tests := []struct {
name string
data []byte
@@ -807,6 +834,21 @@ func TestParseTaggedFields(t *testing.T) {
name: "unknown field valid data",
data: []byte{0xff, 0x00, 0x01, 0xab},
},
+ {
+ name: "malformed then valid payment hash",
+ data: malformedThenValid.Bytes(),
+ wantErr: ErrDuplicatePaymentHash,
+ },
+ {
+ name: "identical payment hashes",
+ data: identicalPaymentHashes.Bytes(),
+ wantErr: ErrDuplicatePaymentHash,
+ },
+ {
+ name: "distinct payment hashes",
+ data: distinctPaymentHashes.Bytes(),
+ wantErr: ErrDuplicatePaymentHash,
+ },
{
name: "only type specified",
data: []byte{0x0d},
### zpay32/invoice_test.go
@@ -211,6 +211,7 @@ func TestDecodeEncode(t *testing.T) {
decodeOpts []DecodeOption
skipEncoding bool
beforeEncoding func(*Invoice)
+ wantErr error
}{
{
encodedInvoice: "asdsaddnasdnas", // no hrp
@@ -340,9 +341,18 @@ func TestDecodeEncode(t *testing.T) {
skipEncoding: true, // Skip encoding since we don't have the unknown fields to encode.
},
{
- // Ignore fields with unknown lengths.
- encodedInvoice: "lnbc241pveeq09pp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqpp3qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqhp58yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqshp38yjmdan79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahnp4q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66np3q0n326hr8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfy8huflvs2zwkymx47cszugvzn5v64ahemzzlmm62rpn9l9rm05h35aceq00tkt296289wepws9jh4499wq2l0vk6xcxffd90dpuqchqqztyayq",
- valid: true,
+ // Ignore fields with unknown lengths. The wrong-length
+ // duplicates of the h and n fields are skipped, while
+ // the valid p, h, and n fields are used.
+ encodedInvoice: "lnbc241pveeq09pp5qqqsyqcyq5rqwzqf" +
+ "qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqhp58yjmdan" +
+ "79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqshp38yjmd" +
+ "an79s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahnp4q0n326hr8v" +
+ "9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66np3q0n326hr8v" +
+ "9zprg8gsvezcch06gfaqqhde2aj730yg0durunfp3ngd7vju6eywrly" +
+ "v9vu7l797m4x5yxvvhqd4rm8guqw5389vna986py0hkxen8kmtmte4d" +
+ "gv439wksk2rh4smnm5w43a0e43lecjvqptnpz74",
+ valid: true,
decodedInvoice: func() *Invoice {
return &Invoice{
Net: &chaincfg.MainNetParams,
@@ -356,6 +366,21 @@ func TestDecodeEncode(t *testing.T) {
},
skipEncoding: true, // Skip encoding since we don't have the unknown fields to encode.
},
+ {
+ // Reject a duplicate payment hash even if it has an
+ // unknown length.
+ encodedInvoice: "lnbc241pveeq09pp5qqqsyqcyq5rqwzqf" +
+ "qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqpp3qqqsyqcyq5rq" +
+ "wzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqhp58yjmdan79s6" +
+ "qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahrqshp38yjmdan79" +
+ "s6qqdhdzgynm4zwqd5d7xmw5fk98klysy043l2ahnp4q0n326hr8v" +
+ "9zprg8gsvezcch06gfaqqhde2aj730yg0durunfhv66np3q0n326h" +
+ "r8v9zprg8gsvezcch06gfaqqhde2aj730yg0durunfy8huflvs2z" +
+ "wkymx47cszugvzn5v64ahemzzlmm62rpn9l9rm05h35aceq00tkt2" +
+ "96289wepws9jh4499wq2l0vk6xcxffd90dpuqchqqztyayq",
+ valid: false,
+ wantErr: ErrDuplicatePaymentHash,
+ },
{
// Invoice with no amount.
encodedInvoice: "lnbc1pvjluezpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jshwlglv23cytkzvq8ld39drs8sq656yh2zn0aevrwu6uqctaklelhtpjnmgjdzmvwsh0kuxuwqf69fjeap9m5mev2qzpp27xfswhs5vgqmn9xzq",
@@ -933,6 +958,9 @@ func TestDecodeEncode(t *testing.T) {
)
if !test.valid {
require.Error(t, err)
+ if test.wantErr != nil {
+ require.ErrorIs(t, err, test.wantErr)
+ }
} else {
require.NoError(t, err)
require.Equal(t, decodedInvoice, invoice)
@@ -963,6 +991,42 @@ func TestDecodeEncode(t *testing.T) {
}
}
+// TestDecodeDuplicatePaymentHashes checks that Decode rejects invoices with
+// either distinct or identical duplicate payment hash fields.
+func TestDecodeDuplicatePaymentHashes(t *testing.T) {
+ t.Parallel()
+
+ tests := map[string]string{
+ "distinct payment hashes": "lnbc1pvjluezpp5qqqsyqcyq5rqwzqf" +
+ "qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqpp5llllll" +
+ "lllllllllllllllllllllllllllllllllllllllllllllsdpy" +
+ "v36hqmrfvdshgefqwpshjmt9de6zq6rpwd5qsp5zyg3zyg3" +
+ "zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygs9g3" +
+ "f93cqturay6zk2fyfcmeflphlzew9wfq0n5nf9hqnlwxtht" +
+ "zqcljcuurljyd2vngkya5hndakf33ghly97qm5nc3umj7j" +
+ "ep22nfsq3nr0w8",
+ "identical payment hashes": "lnbc1pvjluezpp5qqqsyqcyq5rqwzqf" +
+ "qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqpp5qqqsyq" +
+ "cyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqyp" +
+ "qdpyv36hqmrfvdshgefqwpshjmt9de6zq6rpwd5qsp5zyg" +
+ "3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3z" +
+ "ygs29wywgsx0wpv9t045f683nj97nnjk55wt0exe3eassl6" +
+ "smx60nk9hlaae8vhe0hwv25s6fthcwqkxsw2hpjeptxz7x" +
+ "ujtexa3l8jrkcqyn037r",
+ }
+
+ for name, encodedInvoice := range tests {
+ t.Run(name, func(t *testing.T) {
+ t.Parallel()
+
+ _, err := Decode(
+ encodedInvoice, &chaincfg.MainNetParams,
+ )
+ require.ErrorIs(t, err, ErrDuplicatePaymentHash)
+ })
+ }
+}
+
// TestNewInvoice tests that providing the optional arguments to the NewInvoice
// method creates an Invoice that encodes to the expected string.
func TestNewInvoice(t *testing.T) {Risk score
Our methodology →Why this scored 62/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.