← Watch feed
channeld: add extra_tlvs from update_add_htlc msg
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Peter Neuroth
channeld: add extra_tlvs from update_add_htlc msg
We currently only consider known tlv types in the internal
representation of a htlc. This commit adds the remaining unknown tlv
fields to the htlc as well. This is in prepareation to forward these to
the htlc_accepted_hook.
Signed-off-by: Peter Neuroth <pet.v.ne@gmail.com>
✓ Descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit changes how Core Lightning stores extra data fields (called TLVs) attached to HTLC payment messages. Previously, only known fields like the blinded path were kept. Now, any unknown extra TLV fields are preserved inside the internal HTLC record too. The stated purpose is to later forward these extra fields to the htlc_accepted_hook plugin interface. There is no direct security fix here; it is a data-preservation change that could affect how future features or plugins handle payment data.
Recommended action
Treat as a normal feature/data-integrity change. Review the eventual htlc_accepted_hook forwarding code for proper validation of extra TLVs, since preserving unknown fields from peers may expose plugins to malformed or oversized data. No immediate security action is required from this commit alone.
Security signals we found
01
Data field preservation change, not a vulnerability patch
02
Adds memory allocation and copying for attacker-controlled TLV fields
03
Could change plugin-visible behavior once htlc_accepted_hook forwarding is implemented
04
No bounds, validation, or parsing hardening visible in the diff
Evidence from the diff
The patch extends struct htlc with an extra_tlvs field (struct tlv_field array) and updates channel_add_htlc/add_htlc signatures to accept and copy it. In handle_peer_add_htlc, tlvs->fields is now passed into channel_add_htlc alongside the blinded_path. In resend_commitment, stored extra_tlvs are re-serialized into outgoing update_add_htlc messages. Test and fakenet call sites are updated to pass NULL for the new parameter. The commit message explicitly frames this as preparation for forwarding extra TLVs to htlc_accepted_hook.
Changed components
channeld/channeld.cchanneld/channeld_htlc.hchanneld/full_channel.cchanneld/full_channel.hchanneld/test/run-full_channel.ctests/plugins/channeld_fakenet.cInspect captured patch +45 / −13
diff --git a/channeld/channeld.c b/channeld/channeld.c
index 98729dc6..97b4216e 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -621,9 +621,14 @@ static void handle_peer_add_htlc(struct peer *peer, const u8 *msg)
peer_failed_warn(peer->pps, &peer->channel_id,
"Bad peer_add_htlc %s", tal_hex(msg, msg));
}
+
add_err = channel_add_htlc(peer->channel, REMOTE, id, amount,
cltv_expiry, &payment_hash,
- onion_routing_packet, tlvs->blinded_path, &htlc, NULL,
+ onion_routing_packet,
+ take(tlvs->blinded_path), &htlc, NULL,
+ /* NOTE: It might be better to remove the
+ * blinded_path from the extra_tlvs */
+ tlvs->fields,
/* We don't immediately fail incoming htlcs,
* instead we wait and fail them after
* they've been committed */
@@ -5201,13 +5206,24 @@ static void resend_commitment(struct peer *peer, struct changed_htlc *last)
last[i].id);
if (h->state == SENT_ADD_COMMIT) {
- struct tlv_update_add_htlc_tlvs *tlvs;
- if (h->path_key) {
+ struct tlv_update_add_htlc_tlvs *tlvs = NULL;
+ if (h->extra_tlvs || h->path_key) {
tlvs = tlv_update_add_htlc_tlvs_new(tmpctx);
- tlvs->blinded_path = tal_dup(tlvs, struct pubkey,
+ }
+ if (h->extra_tlvs) {
+ tlvs->fields = tal_dup_talarr(tmpctx,
+ struct tlv_field,
+ h->extra_tlvs);
+ }
+ if (h->path_key) {
+ /* It is fine to just set the binded_path
+ * independent of what is in tlv->fields as the
+ * towire logic will serialize unknown fields
+ * and known types seperately. */
+ tlvs->blinded_path = tal_dup(tlvs,
+ struct pubkey,
h->path_key);
- } else
- tlvs = NULL;
+ }
msg = towire_update_add_htlc(NULL, &peer->channel_id,
h->id, h->amount,
&h->rhash,
@@ -6165,7 +6181,7 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
e = channel_add_htlc(peer->channel, LOCAL, peer->htlc_id,
amount, cltv_expiry, &payment_hash,
onion_routing_packet, take(path_key), NULL,
- &htlc_fee, true);
+ &htlc_fee, NULL, true);
status_debug("Adding HTLC %"PRIu64" amount=%s cltv=%u gave %s",
peer->htlc_id,
fmt_amount_msat(tmpctx, amount),
diff --git a/channeld/channeld_htlc.h b/channeld/channeld_htlc.h
index 61258b0a..14d41f07 100644
--- a/channeld/channeld_htlc.h
+++ b/channeld/channeld_htlc.h
@@ -5,6 +5,7 @@
#include <common/htlc.h>
#include <common/pseudorand.h>
#include <wire/onion_wire.h>
+#include <wire/tlvstream.h>
struct htlc {
/* What's the status. */
@@ -29,6 +30,9 @@ struct htlc {
/* Blinding (optional). */
struct pubkey *path_key;
+ /* Any extra tlvs attached to this hltc (optional). */
+ struct tlv_field *extra_tlvs;
+
/* Should we immediately fail this htlc? */
bool fail_immediate;
};
diff --git a/channeld/full_channel.c b/channeld/full_channel.c
index e434bd4c..2bcc4d58 100644
--- a/channeld/full_channel.c
+++ b/channeld/full_channel.c
@@ -588,6 +588,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
struct htlc **htlcp,
bool enforce_aggregate_limits,
struct amount_sat *htlc_fee,
+ struct tlv_field *extra_tlvs,
bool err_immediate_failures)
{
struct htlc *htlc, *old;
@@ -613,6 +614,15 @@ static enum channel_add_err add_htlc(struct channel *channel,
htlc->failed = NULL;
htlc->r = NULL;
htlc->routing = tal_dup_arr(htlc, u8, routing, TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE), 0);
+ if (extra_tlvs && tal_count(extra_tlvs) > 0) {
+ htlc->extra_tlvs = tal_dup_talarr(htlc, struct tlv_field, extra_tlvs);
+ for (size_t i = 0; i < tal_count(extra_tlvs); i++) {
+ /* We need to attach the value to the correct parent */
+ htlc->extra_tlvs[i].value = tal_dup_talarr(htlc, u8, htlc->extra_tlvs[i].value);
+ }
+ } else {
+ htlc->extra_tlvs = NULL;
+ }
/* FIXME: Change expiry to simple u32 */
@@ -905,6 +915,7 @@ enum channel_add_err channel_add_htlc(struct channel *channel,
const struct pubkey *path_key TAKES,
struct htlc **htlcp,
struct amount_sat *htlc_fee,
+ struct tlv_field *extra_tlvs,
bool err_immediate_failures)
{
enum htlc_state state;
@@ -923,7 +934,7 @@ enum channel_add_err channel_add_htlc(struct channel *channel,
return add_htlc(channel, state, id, amount, cltv_expiry,
payment_hash, routing, path_key,
- htlcp, true, htlc_fee, err_immediate_failures);
+ htlcp, true, htlc_fee, extra_tlvs, err_immediate_failures);
}
struct htlc *channel_get_htlc(struct channel *channel, enum side sender, u64 id)
@@ -1621,7 +1632,7 @@ bool channel_force_htlcs(struct channel *channel,
&htlcs[i]->payment_hash,
htlcs[i]->onion_routing_packet,
htlcs[i]->path_key,
- &htlc, false, NULL, false);
+ &htlc, false, NULL, NULL, false);
if (e != CHANNEL_ERR_ADD_OK) {
status_broken("%s HTLC %"PRIu64" failed error %u",
htlc_state_owner(htlcs[i]->state) == LOCAL
diff --git a/channeld/full_channel.h b/channeld/full_channel.h
index 3eaba508..33e54627 100644
--- a/channeld/full_channel.h
+++ b/channeld/full_channel.h
@@ -68,7 +68,6 @@ struct channel *new_full_channel(const tal_t *ctx,
* @remote_splice_amnt: how much is being spliced in (or out, if -ve) of remote side.
* @other_anchor_outnum: which output (-1 if none) is the !!side anchor
* @funding_pubkeys: The funding pubkeys (specify NULL to use channel's value).
- *
* Returns the unsigned commitment transaction for the committed state
* for @side, followed by the htlc transactions in output order and
* fills in @htlc_map, or NULL on key derivation failure.
@@ -115,6 +114,7 @@ u32 actual_feerate(const struct channel *channel,
* @routing: routing information (copied)
* @blinding: optional blinding information for this HTLC.
* @htlcp: optional pointer for resulting htlc: filled in if and only if CHANNEL_ERR_NONE.
+ * @extra_tlvs: optinal tlvs attached to this HTLC.
* @err_immediate_failures: in some cases (dusty htlcs) we want to immediately
* fail the htlc; for peer incoming don't want to
* error, but rather mark it as failed and fail after
@@ -134,6 +134,7 @@ enum channel_add_err channel_add_htlc(struct channel *channel,
const struct pubkey *blinding TAKES,
struct htlc **htlcp,
struct amount_sat *htlc_fee,
+ struct tlv_field *extra_tlvs,
bool err_immediate_failures);
/**
diff --git a/channeld/test/run-full_channel.c b/channeld/test/run-full_channel.c
index c0d41ab7..69128019 100644
--- a/channeld/test/run-full_channel.c
+++ b/channeld/test/run-full_channel.c
@@ -177,7 +177,7 @@ static const struct htlc **include_htlcs(struct channel *channel, enum side side
memset(&preimage, i, sizeof(preimage));
sha256(&hash, &preimage, sizeof(preimage));
e = channel_add_htlc(channel, sender, i, msatoshi, 500+i, &hash,
- dummy_routing, NULL, NULL, NULL, true);
+ dummy_routing, NULL, NULL, NULL, NULL, true);
assert(e == CHANNEL_ERR_ADD_OK);
htlcs[i] = channel_get_htlc(channel, sender, i);
}
@@ -269,7 +269,7 @@ static void send_and_fulfill_htlc(struct channel *channel,
sha256(&rhash, &r, sizeof(r));
assert(channel_add_htlc(channel, sender, 1337, msatoshi, 900, &rhash,
- dummy_routing, NULL, NULL, NULL, true)
+ dummy_routing, NULL, NULL, NULL, NULL, true)
== CHANNEL_ERR_ADD_OK);
htlc = channel_get_htlc(channel, sender, 1337);
assert(htlc);
diff --git a/tests/plugins/channeld_fakenet.c b/tests/plugins/channeld_fakenet.c
index 7b44cf38..5795af23 100644
--- a/tests/plugins/channeld_fakenet.c
+++ b/tests/plugins/channeld_fakenet.c
@@ -893,7 +893,7 @@ static void handle_offer_htlc(struct info *info, const u8 *inmsg)
e = channel_add_htlc(info->channel, LOCAL, htlc->htlc_id,
amount, cltv_expiry, &htlc->payment_hash,
onion_routing_packet, take(blinding), NULL,
- &htlc_fee, true);
+ &htlc_fee, NULL, true);
status_debug("Adding HTLC %"PRIu64" amount=%s cltv=%u gave %s",
htlc->htlc_id, fmt_amount_msat(tmpctx, amount),
cltv_expiry,
Risk score
Our methodology →Why this scored 28/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.