← Watch feed
channeld: Add extra_tlvs to incomming wire msg
Public commit record
68/100 · Adequate
What the developer wrote
Authored by Peter Neuroth
channeld: Add extra_tlvs to incomming wire msg
This appends the extra_tlvs to the internal channeld_offer_htlc wire
msg. We also recombine the extra_tlvs with the blinded path key for
forwarding htlcs.
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 passes extra data fields (TLVs) when forwarding or storing payment instructions inside the channel daemon. It adds a new field to an internal message and recombines those extra fields with a blinding key before the HTLC is added to the channel. There is no direct evidence in the commit that this fixes a security bug; it appears to be a protocol-correctness or feature-completeness change for blinded routes.
Recommended action
Treat as a normal code-review item. Verify that malformed extra_tlvs cannot be used to crash channeld or cause memory corruption, since parsing failure is only logged. Confirm that callers forwarding HTLCs actually populate extra_tlvs when required by protocol specs, because the current callers pass NULL. If this change is part of a larger blinded-route feature, review the full series for consistency.
Security signals we found
01
New internal wire field added (extra_tlvs) and parsed with fromwire_tlv using FROMWIRE_TLV_ANY_TYPE
02
Parsed TLV failure is logged via status_unusual but does not appear to abort HTLC handling
03
Blinded path key and extra TLVs are recombined before channel_add_htlc
04
No explicit bounds/sanity checks on parsed TLV contents beyond the generic TLV parser
05
No mention of CVE, security bug, or vulnerability in commit message or diff
Evidence from the diff
The patch extends the internal channeld_offer_htlc wire message with an extra_tlvs byte vector. In channeld.c, handle_offer_htlc now parses that raw TLV blob using fromwire_tlv with FROMWIRE_TLV_ANY_TYPE, attaches the resulting tlv_field list to the HTLC, and still sets the blinded_path field when a path_key is present. Call sites in lightningd/peer_htlcs.c and the test fakenet are updated to pass NULL for the new field. Wallet test stubs and Makefile dependencies are updated for the new TLV helpers. The change is plumbing: it makes sure arbitrary TLVs received with an HTLC are preserved and combined with the blinded-path key rather than being dropped.
Changed components
channeld/channeld.cchanneld/channeld_wire.csvlightningd/peer_htlcs.ctests/plugins/channeld_fakenet.cwallet/test/Makefilewallet/test/run-wallet.cInspect captured patch +41 / −27
diff --git a/channeld/channeld.c b/channeld/channeld.c
index 2ead09e6..fe7f7707 100644
--- a/channeld/channeld.c
+++ b/channeld/channeld.c
@@ -49,6 +49,7 @@
#include <stdio.h>
#include <wally_bip32.h>
#include <wire/peer_wire.h>
+#include <wire/tlvstream.h>
#include <wire/wire_sync.h>
/* stdin == requests, 3 == peer, 4 = HSM */
@@ -6162,7 +6163,9 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
const char *failstr;
struct amount_sat htlc_fee;
struct pubkey *path_key;
+ struct tlv_field *extra_tlvs;
struct tlv_update_add_htlc_tlvs *tlvs;
+ u8 *extra_tlvs_raw;
if (!peer->channel_ready[LOCAL] || !peer->channel_ready[REMOTE])
status_failed(STATUS_FAIL_MASTER_IO,
@@ -6170,19 +6173,39 @@ static void handle_offer_htlc(struct peer *peer, const u8 *inmsg)
if (!fromwire_channeld_offer_htlc(tmpctx, inmsg, &amount,
&cltv_expiry, &payment_hash,
- onion_routing_packet, &path_key))
+ onion_routing_packet, &path_key, &extra_tlvs_raw))
master_badmsg(WIRE_CHANNELD_OFFER_HTLC, inmsg);
- if (path_key) {
+
+ if (extra_tlvs_raw || path_key) {
tlvs = tlv_update_add_htlc_tlvs_new(tmpctx);
- tlvs->blinded_path = tal_dup(tlvs, struct pubkey, path_key);
- } else
+ } else {
tlvs = NULL;
+ }
+
+ if (extra_tlvs_raw) {
+ const u8 *cursor = extra_tlvs_raw;
+ size_t max = tal_bytelen(extra_tlvs_raw);
+ u64 failedtype;
+ const u64 *allowed = cast_const(u64 *, FROMWIRE_TLV_ANY_TYPE);
+ if (!fromwire_tlv(&cursor, &max, NULL, 0,
+ tlvs, &tlvs->fields,
+ allowed, NULL, &failedtype)) {
+ status_unusual("Malformed TLV type %"PRIu64": %s",
+ failedtype, tal_hex(tmpctx, extra_tlvs_raw));
+ }
+ extra_tlvs = tlvs->fields;
+ } else {
+ extra_tlvs = NULL;
+ }
+ if (path_key) {
+ tlvs->blinded_path = tal_dup(tlvs, struct pubkey, path_key);
+ }
e = channel_add_htlc(peer->channel, LOCAL, peer->htlc_id,
amount, cltv_expiry, &payment_hash,
onion_routing_packet, take(path_key), NULL,
- &htlc_fee, NULL, true);
+ &htlc_fee, extra_tlvs, 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_wire.csv b/channeld/channeld_wire.csv
index 3abacb99..be5a5c6d 100644
--- a/channeld/channeld_wire.csv
+++ b/channeld/channeld_wire.csv
@@ -94,6 +94,8 @@ msgdata,channeld_offer_htlc,cltv_expiry,u32,
msgdata,channeld_offer_htlc,payment_hash,sha256,
msgdata,channeld_offer_htlc,onion_routing_packet,u8,1366
msgdata,channeld_offer_htlc,path_key,?pubkey,
+msgdata,channeld_offer_htlc,extra_tlvs_len,u16,
+msgdata,channeld_offer_htlc,extra_tlvs,u8,extra_tlvs_len
# Reply; synchronous since IDs have to increment.
msgtype,channeld_offer_htlc_reply,1104
diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
index fc4a7063..f21d796b 100644
--- a/lightningd/peer_htlcs.c
+++ b/lightningd/peer_htlcs.c
@@ -701,7 +701,7 @@ const u8 *send_htlc_out(const tal_t *ctx,
struct htlc_in *in,
struct htlc_out **houtp)
{
- u8 *msg;
+ u8 *msg, *raw_tlvs = NULL;
*houtp = NULL;
@@ -743,7 +743,8 @@ const u8 *send_htlc_out(const tal_t *ctx,
}
msg = towire_channeld_offer_htlc(out, amount, cltv, payment_hash,
- onion_routing_packet, path_key);
+ onion_routing_packet, path_key,
+ raw_tlvs);
subd_req(out->peer->ld, out->owner, take(msg), -1, 0, rcvd_htlc_reply,
*houtp);
@@ -2646,7 +2647,7 @@ const struct existing_htlc **peer_htlcs(const tal_t *ctx,
hin->onion_routing_packet,
hin->path_key,
hin->preimage,
- f);
+ f, NULL);
tal_arr_expand(&htlcs, existing);
}
@@ -2678,7 +2679,7 @@ const struct existing_htlc **peer_htlcs(const tal_t *ctx,
hout->onion_routing_packet,
hout->path_key,
hout->preimage,
- f);
+ f, NULL);
tal_arr_expand(&htlcs, existing);
}
diff --git a/tests/plugins/channeld_fakenet.c b/tests/plugins/channeld_fakenet.c
index 5795af23..fc08d2bf 100644
--- a/tests/plugins/channeld_fakenet.c
+++ b/tests/plugins/channeld_fakenet.c
@@ -871,7 +871,7 @@ static void delayed_forward(struct delayed_forward *dfwd)
static void handle_offer_htlc(struct info *info, const u8 *inmsg)
{
- u8 *msg;
+ u8 *msg, *extratlvs;
u32 cltv_expiry;
struct amount_msat amount;
u8 onion_routing_packet[TOTAL_PACKET_SIZE(ROUTING_INFO_SIZE)];
@@ -887,7 +887,7 @@ static void handle_offer_htlc(struct info *info, const u8 *inmsg)
htlc->htlc_id = htlc_id;
if (!fromwire_channeld_offer_htlc(tmpctx, inmsg, &amount,
&cltv_expiry, &htlc->payment_hash,
- onion_routing_packet, &blinding))
+ onion_routing_packet, &blinding, &extratlvs))
master_badmsg(WIRE_CHANNELD_OFFER_HTLC, inmsg);
e = channel_add_htlc(info->channel, LOCAL, htlc->htlc_id,
diff --git a/wallet/test/Makefile b/wallet/test/Makefile
index 14186087..61789189 100644
--- a/wallet/test/Makefile
+++ b/wallet/test/Makefile
@@ -30,9 +30,11 @@ WALLET_TEST_COMMON_OBJS := \
common/utxo.o \
common/wireaddr.o \
common/version.o \
+ common/bigsize.o \
wallet/db_sqlite3_sqlgen.o \
wire/towire.o \
- wire/fromwire.o
+ wire/fromwire.o \
+ wire/tlvstream.o
$(WALLET_TEST_PROGRAMS): $(BITCOIN_OBJS) $(WALLET_TEST_COMMON_OBJS)
$(WALLET_TEST_OBJS): $(WALLET_HDRS) $(WALLET_SRC)
diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
index 881828ae..6f187e72 100644
--- a/wallet/test/run-wallet.c
+++ b/wallet/test/run-wallet.c
@@ -46,9 +46,6 @@ static void test_error(struct lightningd *ld, bool fatal, const char *fmt, va_li
void add_node_announcement_sig(u8 *nannounce UNNEEDED,
const secp256k1_ecdsa_signature *sig UNNEEDED)
{ fprintf(stderr, "add_node_announcement_sig called!\n"); abort(); }
-/* Generated stub for bigsize_put */
-size_t bigsize_put(u8 buf[BIGSIZE_MAX_LEN] UNNEEDED, bigsize_t v UNNEEDED)
-{ fprintf(stderr, "bigsize_put called!\n"); abort(); }
/* Generated stub for bitcoind_getrawblockbyheight_ */
void bitcoind_getrawblockbyheight_(const tal_t *ctx UNNEEDED,
struct bitcoind *bitcoind UNNEEDED,
@@ -368,12 +365,6 @@ bool fromwire_onchaind_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNE
/* Generated stub for fromwire_openingd_dev_memleak_reply */
bool fromwire_openingd_dev_memleak_reply(const void *p UNNEEDED, bool *leak UNNEEDED)
{ fprintf(stderr, "fromwire_openingd_dev_memleak_reply called!\n"); abort(); }
-/* Generated stub for fromwire_tlv */
-bool fromwire_tlv(const u8 **cursor UNNEEDED, size_t *max UNNEEDED,
- const struct tlv_record_type *types UNNEEDED, size_t num_types UNNEEDED,
- void *record UNNEEDED, struct tlv_field **fields UNNEEDED,
- const u64 *extra_types UNNEEDED, size_t *err_off UNNEEDED, u64 *err_type UNNEEDED)
-{ fprintf(stderr, "fromwire_tlv called!\n"); abort(); }
/* Generated stub for get_network_blockheight */
u32 get_network_blockheight(const struct chain_topology *topo UNNEEDED)
{ fprintf(stderr, "get_network_blockheight called!\n"); abort(); }
@@ -1081,7 +1072,7 @@ u8 *towire_channeld_got_commitsig_reply(const tal_t *ctx UNNEEDED)
u8 *towire_channeld_got_revoke_reply(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_channeld_got_revoke_reply called!\n"); abort(); }
/* Generated stub for towire_channeld_offer_htlc */
-u8 *towire_channeld_offer_htlc(const tal_t *ctx UNNEEDED, struct amount_msat amount_msat UNNEEDED, u32 cltv_expiry UNNEEDED, const struct sha256 *payment_hash UNNEEDED, const u8 onion_routing_packet[1366] UNNEEDED, const struct pubkey *path_key UNNEEDED)
+u8 *towire_channeld_offer_htlc(const tal_t *ctx UNNEEDED, struct amount_msat amount_msat UNNEEDED, u32 cltv_expiry UNNEEDED, const struct sha256 *payment_hash UNNEEDED, const u8 onion_routing_packet[1366] UNNEEDED, const struct pubkey *path_key UNNEEDED, const u8 *extra_tlvs UNNEEDED)
{ fprintf(stderr, "towire_channeld_offer_htlc called!\n"); abort(); }
/* Generated stub for towire_channeld_sending_commitsig_reply */
u8 *towire_channeld_sending_commitsig_reply(const tal_t *ctx UNNEEDED)
@@ -1187,11 +1178,6 @@ u8 *towire_temporary_channel_failure(const tal_t *ctx UNNEEDED, const u8 *channe
/* Generated stub for towire_temporary_node_failure */
u8 *towire_temporary_node_failure(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_temporary_node_failure called!\n"); abort(); }
-/* Generated stub for towire_tlv */
-void towire_tlv(u8 **pptr UNNEEDED,
- const struct tlv_record_type *types UNNEEDED, size_t num_types UNNEEDED,
- const void *record UNNEEDED)
-{ fprintf(stderr, "towire_tlv called!\n"); abort(); }
/* Generated stub for towire_unknown_next_peer */
u8 *towire_unknown_next_peer(const tal_t *ctx UNNEEDED)
{ fprintf(stderr, "towire_unknown_next_peer called!\n"); abort(); }
Risk score
Our methodology →Why this scored 26/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.