bolt11: fix crashes and memory safety errors

.github/workflows/ci.yaml

@@ -203,7 +203,7 @@ jobs:
 
       - name: Build
         run: |
-          ./configure --enable-debugbuild --enable-fuzzing --disable-valgrind CC=clang
+          ./configure --enable-debugbuild --enable-fuzzing --enable-address-sanitizer --enable-ub-sanitizer --disable-valgrind CC=clang
           make -j $(nproc) check-fuzz
 
   integration:

common/bolt11.c

@@ -76,7 +76,11 @@ static const char *pull_uint(struct hash_u5 *hu5,
 	err = pull_bits(hu5, data, data_len, &be_val, databits, true);
 	if (err)
 		return err;
-	*val = be64_to_cpu(be_val) >> (sizeof(be_val) * CHAR_BIT - databits);
+	if (databits == 0)
+		*val = 0;
+	else
+		*val = be64_to_cpu(be_val) >>
+		       (sizeof(be_val) * CHAR_BIT - databits);
 	return NULL;
 }
 
@@ -302,14 +306,27 @@ static const char *decode_n(struct bolt11 *b11,
 			    const u5 **data, size_t *field_len,
 			    bool *have_n)
 {
+	const char *err;
+
 	assert(!*have_n);
 	/* BOLT #11:
 	 *
 	 * A reader... MUST skip over unknown fields, OR an `f` field
 	 * with unknown `version`, OR `p`, `h`, `s` or `n` fields that do
 	 * NOT have `data_length`s of 52, 52, 52 or 53, respectively. */
-	return pull_expected_length(b11, hu5, data, field_len, 53, 'n',
-				    have_n, &b11->receiver_id.k);
+	err = pull_expected_length(b11, hu5, data, field_len, 53, 'n', have_n,
+				   &b11->receiver_id.k);
+
+	/* If that gave us a node ID, check it. */
+	if (*have_n) {
+		struct pubkey k;
+		if (!pubkey_from_node_id(&k, &b11->receiver_id))
+			return tal_fmt(
+			    b11, "invalid public key %s",
+			    node_id_to_hexstr(tmpctx, &b11->receiver_id));
+	}
+
+	return err;
 }
 
 /* BOLT #11:
@@ -390,6 +407,8 @@ static const char *decode_f(struct bolt11 *b11,
 		fallback = scriptpubkey_p2sh_hash(b11, shash);
 	} else if (version < 17) {
 		u8 *f = pull_all(tmpctx, hu5, data, field_len, false, &err);
+		if (!f)
+			return err;
 		if (version == 0) {
 			if (tal_count(f) != 20 && tal_count(f) != 32)
 				return tal_fmt(b11,
@@ -715,7 +734,6 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
 	memset(have_field, 0, sizeof(have_field));
 	b11->routes = tal_arr(b11, struct route_info *, 0);
 
-	assert(!has_lightning_prefix(str));
 	if (strlen(str) < 8)
 		return decode_fail(b11, fail, "Bad bech32 string");
 
@@ -918,6 +936,8 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
 	return b11;
 }
 
+static bool valid_recovery_id(u8 recid) { return recid <= 3; }
+
 /* Decodes and checks signature; returns NULL on error. */
 struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
 			     const struct feature_set *our_features,
@@ -958,6 +978,10 @@ struct bolt11 *bolt11_decode(const tal_t *ctx, const char *str,
 
 	assert(data_len == 0);
 
+	if (!valid_recovery_id(sig_and_recid[64]))
+		return decode_fail(b11, fail, "invalid recovery ID: %u",
+				   sig_and_recid[64]);
+
 	if (!secp256k1_ecdsa_recoverable_signature_parse_compact
 	    (secp256k1_ctx, &sig, sig_and_recid, sig_and_recid[64]))
 		return decode_fail(b11, fail, "signature invalid");

tests/fuzz/corpora/fuzz-bolt11/crash-02a760b43a1728ac699ca34f2a01d01cd4e6385f

@@ -0,0 +1 @@
+lnbc1qqygh9qpp5s7zxqqqqqqqqqqqqpjqqqqqqqqqqqqqqqqqqcqpjqqqsqqqqqqqqdqqqqqqqqqqqqqqqqqqqqqqqqqqqqquqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqzxqqqqqqqqqqqqqqqy6f523d

tests/fuzz/corpora/fuzz-bolt11/crash-98a2112c93362e35310c629081b5d60390062962

@@ -0,0 +1 @@
+lightning:

tests/fuzz/corpora/fuzz-bolt11/crash-c4df1aeeec336a31cb468bef279a490cf472acb2

@@ -0,0 +1 @@
+lnbc1qqqqpqqnp4qqqlftcw9qqqqqqqqqqqqygh9qpp5qpp5s7zxqqqqcqpjpqqygh9qpp5s7zxqqqqcqpjpqqlqqqqqqqqqqqqcqqpqqqqqqqqqqqsqqqqqqqqdqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqpqqqqqqqqqqqqqqqqqqqqqqqqqqqqqlqqqcqpjptfqptfqptfqpqqqqqqqqqqqqqqqqqqq8ddm0a

tests/fuzz/fuzz-bolt11.c

@@ -18,11 +18,7 @@ size_t LLVMFuzzerMutate(uint8_t *data, size_t size, size_t max_size);
 size_t LLVMFuzzerCustomMutator(uint8_t *fuzz_data, size_t size, size_t max_size,
 			       unsigned int seed);
 
-void init(int *argc, char ***argv)
-{
-	chainparams = chainparams_for_network("bitcoin");
-	common_setup("fuzzer");
-}
+void init(int *argc, char ***argv) { common_setup("fuzzer"); }
 
 // Encodes a dummy bolt11 invoice into `fuzz_data` and returns the size of the
 // encoded string.
@@ -102,12 +98,6 @@ size_t LLVMFuzzerCustomMutator(uint8_t *fuzz_data, size_t size, size_t max_size,
 		return initial_input(fuzz_data, size, max_size);
 	}
 
-	// Strip (repeated) "lightning:" prefixes
-	while (strstarts(output, "lightning:") ||
-	       strstarts(output, "LIGHTNING:")) {
-		output = (char *)to_canonical_invstr(tmpctx, output);
-	}
-
 	// Write the result into `fuzz_data`.
 	size_t output_len = strlen(output);
 	if (output_len > max_size)
@@ -121,13 +111,9 @@ size_t LLVMFuzzerCustomMutator(uint8_t *fuzz_data, size_t size, size_t max_size,
 void run(const uint8_t *data, size_t size)
 {
 	char *invoice_str = to_string(tmpctx, data, size);
-
-	struct sha256 hash;
-	const u5 *sigdata;
-	bool have_n = false;
 	char *fail;
-	bolt11_decode_nosig(tmpctx, invoice_str, NULL, NULL, chainparams, &hash,
-			    &sigdata, &have_n, &fail);
+
+	bolt11_decode(tmpctx, invoice_str, NULL, NULL, NULL, &fail);
 
 	clean_tmpctx();
 }