decode: fix crash when decoding invalid rune.

If rune contains invalid UTF-8, offers (which implements decode) would
produce JSON with invalid UTF-8, which causes lightningd to complain
and kill it, and then die because it's an important plugin.

So don't decode invalid UTF-8!

Reported-by: @jb55
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

doc/lightning-decode.7.md

@@ -163,7 +163,8 @@ If **type** is "bolt11 invoice", and **valid** is *true*:
     - **tag** (string): The bech32 letter which identifies this field (always 1 characters)
     - **data** (string): The bech32 data for this field
 
-If **type** is "rune":
+If **type** is "rune", and **valid** is *true*:
+  - **valid** (boolean) (always *true*)
   - **string** (string): the string encoding of the rune
   - **restrictions** (array of objects): restrictions built into the rune: all must pass:
     - **alternatives** (array of strings): each way restriction can be met: any can pass:
@@ -171,7 +172,12 @@ If **type** is "rune":
     - **summary** (string): human-readable summary of this restriction
   - **unique_id** (string, optional): unique id (always a numeric id on runes we create)
   - **version** (string, optional): rune version, not currently set on runes we create
-  - **valid** (boolean, optional) (always *true*)
+
+If **type** is "rune", and **valid** is *false*:
+  - **valid** (boolean) (always *false*)
+  - **hex** (hex, optional): the raw rune in hex
+  - the following warnings are possible:
+    - **warning_rune_invalid_utf8**: the rune contains invalid UTF-8 strings
 
 [comment]: # (GENERATE-FROM-SCHEMA-END)
 
@@ -195,4 +201,4 @@ RESOURCES
 
 Main web site: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:d1e1f044c2e67ec169728dbc551903c97f9a9daa1f42e9d2f1686fc692d25be8)
+[comment]: # ( SHA256STAMP:a3963c3e0061b0d42a1f9e2f2a9012df780fce0264c6785f0311909b01f78af2)

doc/schemas/decode.schema.json

@@ -919,13 +919,20 @@
             "enum": [
               "rune"
             ]
+          },
+          "valid": {
+            "type": "boolean",
+            "enum": [
+              true
+            ]
           }
         }
       },
       "then": {
         "required": [
           "string",
-          "restrictions"
+          "restrictions",
+          "valid"
         ],
         "additionalProperties": false,
         "properties": {
@@ -976,6 +983,47 @@
           }
         }
       }
+    },
+    {
+      "if": {
+        "properties": {
+          "type": {
+            "type": "string",
+            "enum": [
+              "rune"
+            ]
+          },
+          "valid": {
+            "type": "boolean",
+            "enum": [
+              false
+            ]
+          }
+        }
+      },
+      "then": {
+        "required": [
+          "valid"
+        ],
+        "additionalProperties": false,
+        "properties": {
+          "valid": {
+            "type": "boolean",
+            "enum": [
+              false
+            ]
+          },
+          "type": {},
+          "warning_rune_invalid_utf8": {
+            "type": "string",
+            "description": "the rune contains invalid UTF-8 strings"
+          },
+          "hex": {
+            "type": "hex",
+            "description": "the raw rune in hex"
+          }
+        }
+      }
     }
   ]
 }

plugins/offers.c

@@ -819,11 +819,26 @@ static void json_add_invoice_request(struct json_stream *js,
 
 static void json_add_rune(struct command *cmd, struct json_stream *js, const struct rune *rune)
 {
+	const char *string;
+
+	/* Simplest to check everything for UTF-8 compliance at once.
+	 * Since separators are | and & (which cannot appear inside
+	 * UTF-8 multichars), if the entire thing is valid UTF-8 then
+	 * each part is. */
+	string = rune_to_string(tmpctx, rune);
+	if (!utf8_check(string, strlen(string))) {
+		json_add_hex(js, "hex", string, strlen(string));
+		json_add_string(js, "warning_rune_invalid_utf8",
+				"Rune contains invalid UTF-8 strings");
+		json_add_bool(js, "valid", false);
+		return;
+	}
+
 	if (rune->unique_id)
 		json_add_string(js, "unique_id", rune->unique_id);
 	if (rune->version)
 		json_add_string(js, "version", rune->version);
-	json_add_string(js, "string", take(rune_to_string(NULL, rune)));
+	json_add_string(js, "string", take(string));
 
 	json_array_start(js, "restrictions");
 	for (size_t i = rune->unique_id ? 1 : 0; i < tal_count(rune->restrs); i++) {

tests/test_plugin.py

@@ -13,6 +13,7 @@ from utils import (
 )
 
 import ast
+import base64
 import concurrent.futures
 import json
 import os
@@ -2806,7 +2807,6 @@ def test_commando_rune(node_factory):
                              'params': params})
 
 
-@pytest.mark.slow_test
 def test_commando_stress(node_factory, executor):
     """Stress test to slam commando with many large queries"""
     nodes = node_factory.get_nodes(5)
@@ -2837,3 +2837,22 @@ def test_commando_stress(node_factory, executor):
 
     # Should have exactly one discard msg from each discard
     nodes[0].daemon.wait_for_logs([r"New cmd from .*, replacing old"] * discards)
+
+
+def test_commando_badrune(node_factory):
+    """Test invalid UTF-8 encodings in rune: used to make us kill the offers plugin which implements decode, as it gave bad utf8!"""
+    l1 = node_factory.get_node()
+    l1.rpc.decode('5zi6-ugA6hC4_XZ0R7snl5IuiQX4ugL4gm9BQKYaKUU9gCZtZXRob2RebGlzdHxtZXRob2ReZ2V0fG1ldGhvZD1zdW1tYXJ5Jm1ldGhvZC9saXN0ZGF0YXN0b3Jl')
+    rune = l1.rpc.commando_rune(restrictions="readonly")
+
+    binrune = base64.urlsafe_b64decode(rune['rune'])
+    # Mangle each part, try decode.  Skip most of the boring chars
+    # (just '|', '&', '#').
+    for i in range(32, len(binrune)):
+        for span in (range(0, 32), (124, 38, 35), range(127, 256)):
+            for c in span:
+                modrune = binrune[:i] + bytes([c]) + binrune[i + 1:]
+                try:
+                    l1.rpc.decode(base64.urlsafe_b64encode(modrune).decode('utf8'))
+                except RpcError:
+                    pass