Skip to content

Conversation

@TheBlueMatt
Copy link
Collaborator

No description provided.

valentinewallace and others added 13 commits April 29, 2025 19:50
This bug was recently surfaced to us by a user who wrote a test where the
sender is attempting to route an MPP payment split across the two channels that
it has with its LSP, where the LSP has a single large channel to the recipient.

Previously this led to a pathfinding failure because our router was not sending
the maximum possible value over the first MPP path it found due to
overestimating the fees needed to cover the following hops. Because the path
that had just been found was not maxxed out, our router assumed that we had
already found enough paths to cover the full payment amount and that we were
finding additional paths for the purpose of redundant path selection. This
caused the router to mark the recipient's only channel as exhausted, with the
intention of choosing more unique paths in future iterations. In reality, this
ended up with the recipient's only channel being disabled and subsequently
failing to find a route entirely.

Update the router to fully utilize the capacity of any paths it finds in this
situation, preventing the "redundant path selection" behavior from kicking in.

Use and `rustfmt` conflicts resolved in:
 * lightning/src/blinded_path/payment.rs

The new test was also updated to avoid APIs not present on 0.1.
When calculating the maximum contribution of a path to a larger
route, we want to ensure we don't overshoot as that might cause us
to violate a maximum value limit.

In 209cb2a, we started by
calculating with `ceil`, which can trigger exactly that, so here we
drop the extra addition, switching us to `floor`.

Found both by the `router` fuzzer as well as the
`generate_large_mpp_routes` test.
`String::truncate` takes a byte index but panics if we split in the
middle of a UTF-8 codepoint. Sadly, in `InvoiceRequest::fields` we
want to tuncate the payer note to a maximum of 512 bytes, which may
be in the middle of a UTF-8 codepoint and can cause panic.

Here we iterate over the bytes in the string until we find one not
in the middle of a UTF-8 codepoint and then split the string there.

Trivial `rustfmt` conflicts resolved in:
 * lightning/src/offers/invoice_request.rs
In the next commit we attempt to verify `InvoiceRequest`s when
fuzzing so that we can test fetching the `InvoiceRequestFields`,
but its useful to allow the verification to succeed more often
first, which we do here.
This should allow us to reach the panic from two commits ago from
the fuzzer.
Ensures `InvoiceReceived` events are not generated multiple times
when `manually_handle_bolt12_invoice` is enabled.
Duplicate events for the same invoice could cause confusion—this
change introduces an idempotency check to prevent that.

Conflicts resolved in:
 * lightning/src/ln/outbound_payment.rs
due to the migration upstream from `max_total_routing_fee_msat` to
a more general config struct.
…3-relnotes

Add release notes and bump version for 0.1.3
v0.1.3 - Apr 30, 2025 - "Routing Unicode in 2025"

Bug Fixes
=========

 * `Event::InvoiceReceived` is now only generated once for each `Bolt12Invoice`
   received matching a pending outbound payment. Previously it would be provided
   each time we received an invoice, which may happen many times if the sender
   sends redundant messages to improve success rates (lightningdevkit#3658).
 * LDK's router now more fully saturates paths which are subject to HTLC
   maximum restrictions after the first hop. In some rare cases this can result
   in finding paths when it would previously spuriously decide it cannot find
   enough diverse paths (lightningdevkit#3707, lightningdevkit#3755).

Security
========

0.1.3 fixes a denial-of-service vulnerability which cause a crash of an
LDK-based node if an attacker has access to a valid `Bolt12Offer` which the
LDK-based node created.
 * A malicious payer which requests a BOLT 12 Invoice from an LDK-based node
   (via the `Bolt12InvoiceRequest` message) can cause the panic of the
   LDK-based node due to the way `String::truncate` handles UTF-8 codepoints.
   The codepath can only be reached once the received `Botlt12InvoiceRequest`
   has been authenticated to be based on a valid `Bolt12Offer` which the same
   LDK-based node issued (lightningdevkit#3747, lightningdevkit#3750).
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Apr 30, 2025

I've assigned @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz April 30, 2025 17:31
@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@TheBlueMatt TheBlueMatt merged commit 1d12d08 into lightningdevkit:0.1-bindings Apr 30, 2025
7 of 25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants