lightningd: make sure we don't send channel_updates for unasked for channels.
What changed, and why it matters
This commit fixes a protocol compliance bug in Core Lightning. When a payment failed and the node sent back an error message, it could attach a 'channel_update' for a different channel than the one the sender had asked to use. The fix makes sure the attached update matches the requested channel, and omits it if not. This mainly prevents routing confusion and fingerprinting rather than direct theft of funds.
Apply the patch. Nodes should upgrade to avoid sending non-compliant channel_updates, which can leak routing information and confuse peers' pathfinding.
Security signals we found
Protocol compliance fix for BOLT 4 channel_update requirement
Prevents disclosure of channel_update for a channel the peer did not ask about
Avoids routing hint / alias leakage when redirecting to an equivalent channel
Could reduce fingerprinting of channel topology or routing preferences
Evidence from the diff
The patch changes channel_update_for_error() to take the incoming HTLC (hin) and compare hin->payload->forward_channel against both the channel’s real scid and local alias before returning a channel_update. If they don’t match—because the node redirected the HTLC to a different channel with the same peer—it returns NULL and no channel_update is included in the failure onion. Callers in peer_htlcs.c are updated to pass hout->in or hin. The change aligns with BOLT 4’s requirement that a returned channel_update must use the short_channel_id from the incoming onion.
Changed components
lightningd/channel.clightningd/channel.hlightningd/peer_htlcs.cInspect captured patch +32 / −7
diff --git a/lightningd/channel.c b/lightningd/channel.c
index 6ca82cb4..2254d051 100644
--- a/lightningd/channel.c
+++ b/lightningd/channel.c
@@ -1302,10 +1302,32 @@ channel_scid_or_local_alias(const struct channel *chan)
return *chan->alias[LOCAL];
}
+
+/* BOLT #4:
+ * An _intermediate hop_ MUST NOT, but the _final node_:
+ *...
+ * - if it returns a `channel_update`:
+ * - MUST set `short_channel_id` to the `short_channel_id` used
+ * by the incoming onion.
+ */
+/* So, if the scid doesn't match (redirect or we chose an equivalent
+ * channel), we simply don't return an update. */
const u8 *channel_update_for_error(const tal_t *ctx,
+ const struct htlc_in *hin,
struct channel *channel)
{
- /* FIXME: Call directly from callers */
+ if (!hin || !hin->payload || !hin->payload->forward_channel)
+ return NULL;
+
+ /* We shouldn't have forwarded to the channel if it wasn't
+ * allowed to use that scid, so we can keep it simple here. */
+ if ((!channel->alias[LOCAL]
+ || !short_channel_id_eq(*hin->payload->forward_channel, *channel->alias[LOCAL]))
+ && (!channel->scid
+ || !short_channel_id_eq(*hin->payload->forward_channel, *channel->scid))) {
+ return NULL;
+ }
+
return channel_gossip_update_for_error(ctx, channel);
}
diff --git a/lightningd/channel.h b/lightningd/channel.h
index b96666e0..13656474 100644
--- a/lightningd/channel.h
+++ b/lightningd/channel.h
@@ -965,7 +965,9 @@ void channel_set_billboard(struct channel *channel, bool perm,
struct htlc_in *channel_has_htlc_in(struct channel *channel);
struct htlc_out *channel_has_htlc_out(struct channel *channel);
+/* hin can be NULL */
const u8 *channel_update_for_error(const tal_t *ctx,
+ const struct htlc_in *hin,
struct channel *channel);
struct amount_msat htlc_max_possible_send(const struct channel *channel);
diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
index 8a0c819f..a9972627 100644
--- a/lightningd/peer_htlcs.c
+++ b/lightningd/peer_htlcs.c
@@ -570,6 +570,7 @@ static void destroy_hout_subd_died(struct htlc_out *hout)
hout->failmsg = towire_temporary_channel_failure(hout,
channel_update_for_error(tmpctx,
+ hout->in,
hout->key.channel));
/* Assign a temporary state (we're about to free it!) so checks
@@ -619,7 +620,7 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU
*/
/* We still append the channel_update (if we have one!) FIXME: provide an option? */
if (fromwire_peektype(failmsg) & UPDATE) {
- const u8 *update = channel_update_for_error(tmpctx, hout->key.channel);
+ const u8 *update = channel_update_for_error(tmpctx, hout->in, hout->key.channel);
towire(&failmsg, update, tal_bytelen(update));
}
hout->failmsg = tal_steal(hout, failmsg);
@@ -725,7 +726,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
log_info(out->log, "Attempt to send HTLC but unowned (%s)",
channel_state_name(out));
return towire_temporary_channel_failure(ctx,
- channel_update_for_error(tmpctx, out));
+ channel_update_for_error(tmpctx, in, out));
}
/* Note: we allow outgoing HTLCs before sync, for fast startup. */
@@ -865,7 +866,7 @@ static void forward_htlc(struct htlc_in *hin,
next->old_feerate_ppm)) {
failmsg = towire_fee_insufficient(tmpctx, hin->msat,
channel_update_for_error(tmpctx,
- next));
+ hin, next));
goto fail;
}
log_info(hin->key.channel->log,
@@ -879,7 +880,7 @@ static void forward_htlc(struct htlc_in *hin,
|| amount_msat_less(amt_to_forward, next->old_htlc_minimum_msat)
|| amount_msat_greater(amt_to_forward, next->old_htlc_maximum_msat)) {
failmsg = towire_temporary_channel_failure(tmpctx,
- channel_update_for_error(tmpctx, next));
+ channel_update_for_error(tmpctx, hin, next));
goto fail;
}
log_info(hin->key.channel->log,
@@ -889,7 +890,7 @@ static void forward_htlc(struct htlc_in *hin,
if (!check_cltv(hin, cltv_expiry, outgoing_cltv_value,
ld->config.cltv_expiry_delta)) {
failmsg = towire_incorrect_cltv_expiry(tmpctx, cltv_expiry,
- channel_update_for_error(tmpctx, next));
+ channel_update_for_error(tmpctx, hin, next));
goto fail;
}
@@ -908,7 +909,7 @@ static void forward_htlc(struct htlc_in *hin,
outgoing_cltv_value,
get_block_height(ld->topology));
failmsg = towire_expiry_too_soon(tmpctx,
- channel_update_for_error(tmpctx, next));
+ channel_update_for_error(tmpctx, hin, next));
goto fail;
}
Why this scored 40/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.