Skip to content

Commit a780004

Browse files
rustyrussellShahanaFarooqui
authored andcommitted
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 <[email protected]>
1 parent 8879df7 commit a780004

File tree

2 files changed

+8
-8
lines changed

2 files changed

+8
-8
lines changed

lightningd/pay.c

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -588,20 +588,21 @@ void payment_failed(struct lightningd *ld,
588588
onion_wire_name(fail->failcode));
589589
failstr = localfail;
590590
pay_errcode = PAY_TRY_OTHER_ROUTE;
591-
} else if (payment->path_secrets == NULL) {
592-
/* This was a payment initiated with `sendonion`/`injectonionmessage`, we therefore
593-
* don't have the path secrets and cannot decode the error
594-
* onion. We hand it to the user. */
595-
pay_errcode = PAY_UNPARSEABLE_ONION;
596-
fail = NULL;
597-
failstr = NULL;
598591
} else if (failmsg) {
599592
/* This can happen when a direct peer told channeld it's a
600593
* malformed onion using update_fail_malformed_htlc. */
601594
failstr = "local failure";
602595
origin_index = 0;
603596
pay_errcode = PAY_TRY_OTHER_ROUTE;
604597
goto use_failmsg;
598+
} else if (payment->path_secrets == NULL) {
599+
/* This was a payment initiated with `sendonion`/`injectonionmessage`, we therefore
600+
* don't have the path secrets and cannot decode the error
601+
* onion. We hand it to the user. */
602+
assert(failonion != NULL);
603+
pay_errcode = PAY_UNPARSEABLE_ONION;
604+
fail = NULL;
605+
failstr = NULL;
605606
} else {
606607
/* Must be normal remote fail with an onion-wrapped error. */
607608
failstr = "reply from remote";

tests/test_misc.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2116,7 +2116,6 @@ def test_bad_onion(node_factory, bitcoind):
21162116
assert err.value.error['data']['erring_channel'] == route[1]['channel']
21172117

21182118

2119-
@pytest.mark.xfail(strict=True)
21202119
def test_bad_onion_immediate_peer(node_factory, bitcoind):
21212120
"""Test that we handle the malformed msg when we're the origin"""
21222121
l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-fail-process-onionpacket': None}])

0 commit comments

Comments
 (0)