offers: fix recurrence proportional amount - #9413
Conversation
Reported-by: Vincenzo Palazzo (Bitcoin Security Council finding 2026-08-11) Changelog-Fixed: offers: recurrence with proportional_amount now computes the correct invoice amount based on the time remaining Signed-off-by: Lagrang3 <lagrang3@protonmail.com>
9ba7b32 to
c0e2e19
Compare
| if (*ir->inv->invoice_created_at >= end) { | ||
| *ir->inv->invoice_amount = 1; |
There was a problem hiding this comment.
paywindow_end is pstart + seconds_after (common/bolt12.c:513) and seconds_after is an unconstrained u32, so an offer whose paywindow outlives its period reaches this branch. Past the period end the time remaining is zero, so the request should be rejected as too late rather than priced
at 1 msat.
| invreq_recurrence(ir->invreq)); | ||
|
|
||
| if (*ir->inv->invoice_created_at > start) { | ||
| assert(end > start); |
There was a problem hiding this comment.
Please make this a fail_invreq() rather than an assert(). offer_period_start() gives no guarantee that end > start. param_recurrence() (plugins/offers_offer.c:96) has no non-zero check on period, and the period == 0 guard at common/bolt12.c:290 is in offer_decode(), not invrequest_decode(), so a recurrence=0seconds offer reaches here with start == end. period_idx is also invreq_recurrence_start + invreq_recurrence_counter, i.e. remote-chosen, and offer_period_start() truncates period*n into time_change()'s u32 number (common/bolt12.c:449), so the sequence is not monotone across that range either. offers is a builtin and so important, and NDEBUG is never defined, so an abort here
takes the node down.
| assert(end > start); | |
| if (end <= start) | |
| return fail_invreq(cmd, ir, | |
| "period_index %"PRIu64" bad period", | |
| period_idx); |
3 participants
offers: recurrence with proportional_amount now computes the correct invoice amount based on the time remaining