lightningd: handle unparsable onion from first hop when using injectonionmessage.
What the developer wrote
Authored by Rusty Russell
lightningd: handle unparsable onion from first hop when using injectonionmessage.
In this case we have a failmsg, so we should use that. Otherwise we can have
both failmsg and failonion NULL in the call to injectonion_fail, which is not
valid.
```
DEBUG 022d223620a359a47ff7f7ac447c85c46c923da53389221a0054c11c1e3ca31d59-chan#1: Removing out HTLC 1 state RCVD_REMOVE_ACK_REVOCATION WIRE_INVALID_ONION_HMAC
**BROKEN** lightningd: FATAL SIGNAL 11 (version v25.09-135-g19a3bbc-modded)
**BROKEN** lightningd: backtrace: common/daemon.c:41 (send_backtrace) 0x6220e8fe0080
**BROKEN** lightningd: backtrace: common/daemon.c:78 (crashdump) 0x6220e8fe00cf
**BROKEN** lightningd: backtrace: ./signal/../sysdeps/unix/sysv/linux/x86_64/libc_sigaction.c:0 ((null)) 0x73614bc4532f
**BROKEN** lightningd: backtrace: lightningd/pay.c:1701 (injectonion_fail) 0x6220e8f951c0
**BROKEN** lightningd: backtrace: lightningd/pay.c:330 (tell_waiters_failed) 0x6220e8f943be
**BROKEN** lightningd: backtrace: lightningd/pay.c:656 (payment_failed) 0x6220e8f98db1
**BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:313 (fail_out_htlc) 0x6220e8fa1d04
**BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:1988 (remove_htlc_out) 0x6220e8fa271b
**BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:2086 (update_out_htlc) 0x6220e8fa2904
**BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:2095 (changed_htlc) 0x6220e8fa2c24
**BROKEN** lightningd: backtrace: lightningd/peer_htlcs.c:2608 (peer_got_revoke) 0x6220e8fa6e5a
**BROKEN** lightningd: backtrace: lightningd/channel_control.c:1555 (channel_msg) 0x6220e8f62725
**BROKEN** lightningd: backtrace: lightningd/subd.c:560 (sd_msg_read) 0x6220e8fb2eed
**BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:60 (next_plan) 0x6220e90a3335
**BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:422 (do_plan) 0x6220e90a3806
**BROKEN** lightningd: backtrace: ccan/ccan/io/io.c:439 (io_ready) 0x6220e90a38c3
**BROKEN** lightningd: backtrace: ccan/ccan/io/poll.c:455 (io_loop) 0x6220e90a524f
**BROKEN** lightningd: backtrace: lightningd/io_loop_with_timers.c:22 (io_loop_with_timers) 0x6220e8f7d1c7
**BROKEN** lightningd: backtrace: lightningd/lightningd.c:1496 (main) 0x6220e8f82db2
**BROKEN** lightningd: backtrace: ../sysdeps/nptl/libc_start_call_main.h:58 (__libc_start_call_main) 0x73614bc2a1c9
**BROKEN** lightningd: backtrace: ../csu/libc-start.c:360 (__libc_start_main_impl) 0x73614bc2a28a
**BROKEN** lightningd: backtrace: (null):0 ((null)) 0x6220e8f53b64
**BROKEN** lightningd: backtrace: (null):0 ((null)) 0xffffffffffffffff
```
Reported-by: @michael1011
Changelog-Fixed: lightningd: potential crash when we receive a malformed onion complain from our first peer when using sendonion / injectpaymentonion.
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
What changed, and why it matters
This fix stops Core Lightning from crashing with a fatal signal 11 (segfault) when a payment sent via the low-level `sendonion`/`injectpaymentonion` path fails because the very first peer reports a malformed onion. Before the fix, the code could call an internal failure-handling routine with both the failure message and the failure onion set to NULL, which is not allowed and caused the daemon to crash. The patch reorders the logic so the malformed-onion message is used when available, and adds a safety assertion that the onion is present in the remaining case.
Apply the patch and run the now-enabled `test_bad_onion_immediate_peer` regression test. Node operators using `sendonion`/`injectpaymentonion` should upgrade to a release containing this fix to avoid denial-of-service crashes from malformed first-hop onions.
Security signals we found
NULL pointer dereference / segmentation fault in payment failure path
Crash triggered by malformed-onion report from first hop
Logic branch ordering allowed both failmsg and failonion to be NULL simultaneously
Fix reorders conditional checks and adds an assertion for the remaining invariant
Previously failing test is now enabled (xfail removed)
Evidence from the diff
In lightningd/pay.c::payment_failed(), the branch for payment->path_secrets == NULL (used by sendonion/injectonionmessage) was evaluated before the failmsg branch. When the immediate peer rejects the onion via update_fail_malformed_htlc, failmsg is set but failonion is NULL. The original code first entered the path_secrets == NULL branch, setting fail = NULL and failstr = NULL, then later called injectonion_fail() with both failmsg and failonion NULL, triggering a NULL dereference/segfault. The patch moves the failmsg branch ahead of the path_secrets == NULL branch, so the malformed-HTLC message is consumed first, and adds assert(failonion != NULL) in the path_secrets == NULL branch to document and enforce the invariant. The test test_bad_onion_immediate_peer is no longer marked as expected to fail.
Changed components
lightningd/pay.ctests/test_misc.pysendonion / injectpaymentonion RPC flowsHTLC failure handling (payment_failed, injectonion_fail)Inspect captured patch +8 / −8
diff --git a/lightningd/pay.c b/lightningd/pay.c
index 44931d70..bd85e732 100644
--- a/lightningd/pay.c
+++ b/lightningd/pay.c
@@ -588,13 +588,6 @@ void payment_failed(struct lightningd *ld,
onion_wire_name(fail->failcode));
failstr = localfail;
pay_errcode = PAY_TRY_OTHER_ROUTE;
- } else if (payment->path_secrets == NULL) {
- /* This was a payment initiated with `sendonion`/`injectonionmessage`, we therefore
- * don't have the path secrets and cannot decode the error
- * onion. We hand it to the user. */
- pay_errcode = PAY_UNPARSEABLE_ONION;
- fail = NULL;
- failstr = NULL;
} else if (failmsg) {
/* This can happen when a direct peer told channeld it's a
* malformed onion using update_fail_malformed_htlc. */
@@ -602,6 +595,14 @@ void payment_failed(struct lightningd *ld,
origin_index = 0;
pay_errcode = PAY_TRY_OTHER_ROUTE;
goto use_failmsg;
+ } else if (payment->path_secrets == NULL) {
+ /* This was a payment initiated with `sendonion`/`injectonionmessage`, we therefore
+ * don't have the path secrets and cannot decode the error
+ * onion. We hand it to the user. */
+ assert(failonion != NULL);
+ pay_errcode = PAY_UNPARSEABLE_ONION;
+ fail = NULL;
+ failstr = NULL;
} else {
/* Must be normal remote fail with an onion-wrapped error. */
failstr = "reply from remote";
diff --git a/tests/test_misc.py b/tests/test_misc.py
index 4508cf0f..a4d355b1 100644
--- a/tests/test_misc.py
+++ b/tests/test_misc.py
@@ -2119,7 +2119,6 @@ def test_bad_onion(node_factory, bitcoind):
assert err.value.error['data']['erring_channel'] == route[1]['channel']
-@pytest.mark.xfail(strict=True)
def test_bad_onion_immediate_peer(node_factory, bitcoind):
"""Test that we handle the malformed msg when we're the origin"""
l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-fail-process-onionpacket': None}])
Why this scored 64/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.