diff --git a/common/bolt11.c b/common/bolt11.c index bc851ed49114..752c282c3e19 100644 --- a/common/bolt11.c +++ b/common/bolt11.c @@ -1418,3 +1418,23 @@ char *bolt11_encode_(const tal_t *ctx, return output; } + +bool bolt11_route_hints_have_loops(const struct bolt11 *b11) +{ + const struct node_id *current, *next; + for (size_t i = 0; i < tal_count(b11->routes); i++) { + const struct route_info *route = b11->routes[i]; + for (size_t j = 0; j < tal_count(route); j++) { + current = &route[j].pubkey; + + if (j + 1 == tal_count(route)) + next = &b11->receiver_id; + else + next = &route[j + 1].pubkey; + + if (node_id_eq(current, next)) + return true; + } + } + return false; +} diff --git a/common/bolt11.h b/common/bolt11.h index 9c5336758d79..11ce24df8422 100644 --- a/common/bolt11.h +++ b/common/bolt11.h @@ -135,4 +135,8 @@ const char *to_canonical_invstr(const tal_t *ctx, const char *invstring); extern bool dev_bolt11_old_order; extern bool dev_bolt11_omit_c_value; +/* Check if a bolt11 invoice contains self-loops. This doesn't make the invoice + * invalid, but it is not useful and it could be even a malicious intention. */ +bool bolt11_route_hints_have_loops(const struct bolt11 *b11); + #endif /* LIGHTNING_COMMON_BOLT11_H */ diff --git a/plugins/askrene/askrene.c b/plugins/askrene/askrene.c index 490e1ce4cd34..2a328762e4d4 100644 --- a/plugins/askrene/askrene.c +++ b/plugins/askrene/askrene.c @@ -1066,6 +1066,10 @@ static struct command_result *json_askrene_create_channel(struct command *cmd, plugin_log(cmd->plugin, LOG_TRACE, "%s called: %.*s", __func__, json_tok_full_len(params), json_tok_full(buffer, params)); + if (node_id_eq(src, dst)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "source and destination must be different"); + if (layer_find_local_channel(layer, *scid)) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "channel already exists"); diff --git a/plugins/pay.c b/plugins/pay.c index f13d5ff93cce..9f6fd4c66d52 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -944,6 +944,10 @@ static struct command_result *json_pay(struct command *cmd, if (b11 == NULL) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid bolt11: %s", b11_fail); + if (bolt11_route_hints_have_loops(b11)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Route hints contain self-loops " + "in this bolt11 invoice"); invmsat = b11->msat; invexpiry = b11->timestamp + b11->expiry; diff --git a/plugins/renepay/main.c b/plugins/renepay/main.c index 71276041b175..1732ef59ec54 100644 --- a/plugins/renepay/main.c +++ b/plugins/renepay/main.c @@ -118,6 +118,10 @@ static struct command_result *json_renepaystatus(struct command *cmd, if (b11 == NULL) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid bolt11: %s", fail); + if (bolt11_route_hints_have_loops(b11)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Route hints contain self-loops " + "in this bolt11 invoice"); struct payment *payment = payment_map_get(pay_plugin->payment_map, b11->payment_hash); diff --git a/plugins/xpay/xpay.c b/plugins/xpay/xpay.c index 6a0ff3093876..2c3ef5006afa 100644 --- a/plugins/xpay/xpay.c +++ b/plugins/xpay/xpay.c @@ -2699,6 +2699,10 @@ static struct command_result *xpay_core(struct command *cmd, return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "%s", err); + if (bolt11_route_hints_have_loops(b11)) + return command_fail(cmd, JSONRPC2_INVALID_PARAMS, + "Route hints contain self-loops " + "in this bolt11 invoice"); payment->route_hints = tal_steal(payment, b11->routes); disable_mpp = !feature_offered(b11->features, OPT_BASIC_MPP); if (!includefees_msat && amount_msat_is_zero(payment->mpp_amount)) diff --git a/tests/test_askrene.py b/tests/test_askrene.py index 9fbc9e708672..a14b03a521a7 100644 --- a/tests/test_askrene.py +++ b/tests/test_askrene.py @@ -2667,3 +2667,39 @@ def test_impossible_payment(node_factory): final_cltv=5, maxparts=1, ) + + +def test_sanitize_create_channel(node_factory): + """To avoid gossmap's API to crash askrene we should sanitaze user's input. + In particular the obvious bad self-loop channel.""" + gsfile, nodemap = generate_gossip_store([GenChannel(0, 1)]) + l1 = node_factory.get_node(gossip_store_file=gsfile.name) + LAYER = "test-layer" + l1.rpc.askrene_create_layer(LAYER) + + rpcerror = None + try: + l1.rpc.askrene_create_channel( + LAYER, nodemap[0], nodemap[0], "1x1x1", "1000000sat" + ) + except RpcError as e: + # we check later + rpcerror = e + + # This shouldn't crash askrene + l1.rpc.getroutes( + source=nodemap[0], + destination=nodemap[1], + amount_msat=10000, + layers=[LAYER], + maxfee_msat=1000, + final_cltv=99, + ) + + # askrene should refuse to create such channel + with pytest.raises( + RpcError, + match=r"source and destination must be different", + ): + if rpcerror: + raise rpcerror diff --git a/tests/test_xpay.py b/tests/test_xpay.py index c419589ecc9c..6d499e22b183 100644 --- a/tests/test_xpay.py +++ b/tests/test_xpay.py @@ -1359,3 +1359,33 @@ def test_sendamount_bip353(node_factory): ret = l2.rpc.sendamount("fake@fake.com", "100sat") assert ret["successful_parts"] == 1 assert ret["amount_sent_msat"] == 100000 + + +def test_paying_bad_bolt11(node_factory, chainparams): + """A bad bolt11 invoice containing a self-loop routehint should be detected + by xpay.""" + l1, l2, l3 = node_factory.get_nodes(3) + node_factory.join_nodes([l1, l2], wait_for_announce=True) + + self_loop_route = [ + { + "id": l3.info["id"], + "short_channel_id": "1x1x1", + "fee_base_msat": 0, + "fee_proportional_millionths": 0, + "cltv_expiry_delta": 6, + } + ] + inv = l3.dev_invoice( + amount_msat="21sat", + label="bad-invoice", + description="bad-invoice", + dev_routes=[self_loop_route], + )["bolt11"] + + # xpay should refuse to pay this bad invoice + with pytest.raises( + RpcError, + match=r"Route hints contain self-loops", + ): + l1.rpc.xpay(inv)