common/bolt11: meet the new tighter parsing requirements.

These checks are a SHOULD, but implementing them helps avoid anyone
making such weird things in future.

Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

Makefile

@@ -26,7 +26,7 @@ CCANDIR := ccan
 
 # Where we keep the BOLT RFCs
 BOLTDIR := ../bolts/
-DEFAULT_BOLTVERSION := 216914d492ecdf8a814bbab34fa9f6d31b394b98
+DEFAULT_BOLTVERSION := 011bf84d74d130c2972becca97c87f297b9d4a92
 # Can be overridden on cmdline.
 BOLTVERSION := $(DEFAULT_BOLTVERSION)
 

common/bolt11.c

@@ -64,15 +64,21 @@ static const char *pull_bits(struct hash_u5 *hu5,
 
 /* Helper for pulling a variable-length big-endian int. */
 static const char *pull_uint(struct hash_u5 *hu5,
-		      const u5 **data, size_t *data_len,
-		      u64 *val, size_t databits)
+			     const u5 **data, size_t *data_len,
+			     u64 *val, size_t databits,
+			     bool must_be_minimal)
 {
 	be64 be_val;
 	const char *err;
 
 	/* Too big. */
 	if (databits > sizeof(be_val) * CHAR_BIT)
 		return "integer too large";
+	if (must_be_minimal
+	    && databits >= 5
+	    && *data_len > 1
+	    && (*data)[0] == 0)
+		return "not a minimal value";
 	err = pull_bits(hu5, data, data_len, &be_val, databits, true);
 	if (err)
 		return err;
@@ -262,8 +268,12 @@ static const char *decode_x(struct bolt11 *b11,
 
 	assert(!*have_x);
 
-	/* FIXME: Put upper limit in bolt 11 */
-	err = pull_uint(hu5, data, field_len, &b11->expiry, *field_len * 5);
+	/* BOLT #11:
+	 * - if a `c`, `x`, or `9` field is provided which has a non-minimal `data_length`
+	 *   (i.e. begins with 0 field elements):
+	 *   - SHOULD treat the invoice as invalid.
+	 */
+	err = pull_uint(hu5, data, field_len, &b11->expiry, *field_len * 5, true);
 	if (err)
 		return tal_fmt(b11, "x: %s", err);
 
@@ -287,8 +297,12 @@ static const char *decode_c(struct bolt11 *b11,
 
 	assert(!*have_c);
 
-	/* FIXME: Put upper limit in bolt 11 */
-	err = pull_uint(hu5, data, field_len, &c, *field_len * 5);
+	/* BOLT #11:
+	 * - if a `c`, `x`, or `9` field is provided which has a non-minimal `data_length`
+	 *   (i.e. begins with 0 field elements):
+	 *   - SHOULD treat the invoice as invalid.
+	 */
+	err = pull_uint(hu5, data, field_len, &c, *field_len * 5, true);
 	if (err)
 		return tal_fmt(b11, "c: %s", err);
 	b11->min_final_cltv_expiry = c;
@@ -375,7 +389,7 @@ static const char *decode_f(struct bolt11 *b11,
 	size_t orig_len = *field_len;
 	const char *err;
 
-	err = pull_uint(hu5, data, field_len, &version, 5);
+	err = pull_uint(hu5, data, field_len, &version, 5, false);
 	if (err)
 		return tal_fmt(b11, "f: %s", err);
 
@@ -550,6 +564,14 @@ static const char *decode_9(struct bolt11 *b11,
 	if (!b11->features)
 		return err;
 
+	/* BOLT #11:
+	 *   - if a `c`, `x`, or `9` field is provided which has a non-minimal `data_length`
+	 *     (i.e. begins with 0 field elements):
+	 *   - SHOULD treat the invoice as invalid.
+	 */
+	if (tal_count(b11->features) > 0 && b11->features[0] == 0)
+		return tal_fmt(b11, "9: non-minimal length (begins with 0)");
+
 	/* pull_bits pads with zero bits: we need to remove them. */
 	shift_bitmap_down(b11->features,
 			  flen * 8 - databits);
@@ -847,7 +869,7 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
 	 * 1. zero or more tagged parts
 	 * 1. `signature`: Bitcoin-style signature of above (520 bits)
 	 */
-	err = pull_uint(&hu5, &data, &data_len, &b11->timestamp, 35);
+	err = pull_uint(&hu5, &data, &data_len, &b11->timestamp, 35, false);
 	if (err)
 		return decode_fail(b11, fail,
 				   "Can't get 35-bit timestamp: %s", err);
@@ -866,11 +888,11 @@ struct bolt11 *bolt11_decode_nosig(const tal_t *ctx, const char *str,
 		 * 1. `data_length` (10 bits, big-endian)
 		 * 1. `data` (`data_length` x 5 bits)
 		 */
-		err = pull_uint(&hu5, &data, &data_len, &type, 5);
+		err = pull_uint(&hu5, &data, &data_len, &type, 5, false);
 		if (err)
 			return decode_fail(b11, fail,
 					   "Can't get tag: %s", err);
-		err = pull_uint(&hu5, &data, &data_len, &field_len64, 10);
+		err = pull_uint(&hu5, &data, &data_len, &field_len64, 10, false);
 		if (err)
 			return decode_fail(b11, fail,
 					   "Can't get length: %s", err);
@@ -1053,10 +1075,11 @@ static void push_field(u5 **data, char type, const void *src, size_t nbits)
 /* BOLT #11:
  *
  * - if `x` is included:
- *   - SHOULD use the minimum `data_length` possible.
+ *   - MUST use the minimum `data_length` possible, i.e. no leading 0 field-elements.
  * - SHOULD include one `c` field (`min_final_cltv_expiry_delta`).
- *...
- *   - SHOULD use the minimum `data_length` possible.
+ *    - MUST set `c` to the minimum `cltv_expiry` it will accept for the last
+ *      HTLC in the route.
+ *    - MUST use the minimum `data_length` possible, i.e. no leading 0 field-elements.
  */
 static void push_varlen_field(u5 **data, char type, u64 val)
 {
@@ -1197,7 +1220,8 @@ static void maybe_encode_9(u5 **data, const u8 *features,
 	/* BOLT #11:
 	 *
 	 * - if `9` contains non-zero bits:
-	 *   - SHOULD use the minimum `data_length` possible.
+	 *   - MUST use the minimum `data_length` possible to encode the non-zero bits
+	 *     with no 0 field-elements at the start.
 	 * - otherwise:
 	 *   - MUST omit the `9` field altogether.
 	 */