From 9ce625c05d97e15a92465e929d0afcd57753cd52 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 13 Oct 2025 11:50:15 +1030 Subject: [PATCH 1/2] pytest: test for malformed reply from first hop when using injectpaymentonion. Signed-off-by: Rusty Russell --- tests/test_misc.py | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/tests/test_misc.py b/tests/test_misc.py index 0a6aba28dabe..4508cf0f36a0 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -8,10 +8,11 @@ from threading import Event from pyln.testing.utils import ( TIMEOUT, VALGRIND, sync_blockheight, only_one, - wait_for, TailableProc, env, mine_funding_to_announce + wait_for, TailableProc, env, mine_funding_to_announce, ) from utils import ( - account_balance, scriptpubkey_addr, check_coin_moves, first_scid + account_balance, scriptpubkey_addr, check_coin_moves, first_scid, + serialize_payload_tlv, serialize_payload_final_tlv, ) import copy @@ -2118,9 +2119,10 @@ 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}) + l1, l2 = node_factory.line_graph(2, opts=[{}, {'dev-fail-process-onionpacket': None}]) inv = l2.rpc.invoice(123000, 'test_bad_onion_immediate_peer', 'description') route = l1.rpc.getroute(l2.info['id'], 123000, 1)['route'] @@ -2137,6 +2139,26 @@ def test_bad_onion_immediate_peer(node_factory, bitcoind): WIRE_INVALID_ONION_HMAC = 0x8000 | 0x4000 | 5 assert err.value.error['data']['failcode'] == WIRE_INVALID_ONION_HMAC + # Same, but using injectpaymentonion with corrupt onion. + blockheight = l1.rpc.getinfo()['blockheight'] + hops = [{'pubkey': l1.info['id'], + 'payload': serialize_payload_tlv(123000, 18 + 6, first_scid(l1, l2), blockheight).hex()}, + {'pubkey': l2.info['id'], + 'payload': serialize_payload_final_tlv(123000, 18, 123000, blockheight, inv['payment_secret']).hex()}] + onion = l1.rpc.createonion(hops=hops, assocdata=inv['payment_hash']) + + with pytest.raises(RpcError) as err: + l1.rpc.injectpaymentonion(onion=onion['onion'], + payment_hash=inv['payment_hash'], + amount_msat=123000, + cltv_expiry=blockheight + 18 + 6, + partid=1, + groupid=0) + # FIXME: PAY_INJECTPAYMENTONION_FAILED = 218 + PAY_INJECTPAYMENTONION_FAILED = 218 + assert err.value.error['code'] == PAY_INJECTPAYMENTONION_FAILED + assert 'onionreply' in err.value.error['data'] + def test_newaddr(node_factory, chainparams): l1 = node_factory.get_node() From ad76c93caf9a3f020b2b0fdf925bf7abdb006311 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 13 Oct 2025 11:54:04 +1030 Subject: [PATCH 2/2] 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 --- lightningd/pay.c | 15 ++++++++------- tests/test_misc.py | 1 - 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 44931d700ea3..bd85e732d597 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 4508cf0f36a0..a4d355b12b72 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}])