Xpay bad routehints

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;
+}

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 */

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");

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;

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);

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))

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

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)