bolt11 decode accepts a duplicate p or s field and keeps the first

Inserisci il test dell’issue in common/test/run-bolt11-duplicate-p-s.c ed esegui make unittest/common/test/run-bolt11-duplicate-p-s per confermare i tre fallimenti. Poi leggi il ciclo dei campi taggati in common/bolt11.c intorno alla riga 924, dove una ripetizione finisce nel ramo have_field[type] && !decoder->allow_duplicates e viene deviata su unknown_field; controlla come decode segnala il fallimento per altre fatture non valide. Fatto significa che tutte e tre le fatture non riescono a essere decodificate con una stringa di errore, mentre gli esempi BOLT 11 validi in common/test/run-bolt11.c continuano a passare.

Issue and Steps to Reproduce

bolt11_decode_nosig accepts an invoice with two distinct p fields, the same p twice, or two distinct s fields, and keeps the first copy. This happens on master at 1ebc4943 and in v26.06.9.

This test goes in common/test/run-bolt11-duplicate-p-s.c. It signs an otherwise valid invoice with the BOLT 11 example key. It adds a second p or s field, matching the three invalid vectors in the spec.

#include "config.h"
#include "../amount.c"
#include "../bech32.c"
#include "../bech32_util.c"
#include "../bolt11.c"
#include "../features.c"
#include "../node_id.c"
#include "../hash_u5.c"
#include "../wire/fromwire.c"
#include "../wire/towire.c"
#include <ccan/err/err.h>
#include <ccan/str/str.h>
#include <common/setup.h>
#include <stdio.h>

/* AUTOGENERATED MOCKS START */
/* Generated stub for siphash_seed */
const struct siphash_seed *siphash_seed(void)
{ fprintf(stderr, "siphash_seed called!\n"); abort(); }
/* AUTOGENERATED MOCKS END */

static struct privkey privkey;

/* As in run-bolt11.c: sign with the BOLT #11 example key. */
static bool test_sign(const u5 *u5bytes,
		      const u8 *hrpu8,
		      secp256k1_ecdsa_recoverable_signature *rsig,
		      void *unused UNUSED)
{
	struct hash_u5 hu5;
	char *hrp;
	struct sha256 sha;

	hrp = tal_dup_arr(NULL, char, (char *)hrpu8, tal_count(hrpu8), 1);
	hrp[tal_count(hrpu8)] = '\0';

	hash_u5_init(&hu5, hrp);
	hash_u5(&hu5, u5bytes, tal_count(u5bytes));
	hash_u5_done(&hu5, &sha);
	tal_free(hrp);

	if (!secp256k1_ecdsa_sign_recoverable(secp256k1_ctx, rsig,
					      (const u8 *)&sha,
					      privkey.secret.data,
					      NULL, NULL))
		abort();

	return true;
}

/* BOLT #11:
 *
 * > ### Please send $3 for a cup of coffee to the same peer, within one minute
 *
 * We take that example's fields and sign them with the example key.  The first
 * `p` is payment_hash 0001..0102 and the first `s` is secret 0x1111..1111.
 */
static struct bolt11 *coffee(void)
{
	struct amount_msat msat = AMOUNT_MSAT(2500 * (1000ULL * 100000000) / 1000000);
	struct bolt11 *b11 = new_bolt11(tmpctx, &msat);

	b11->chain = chainparams_for_network("bitcoin");
	b11->timestamp = 1496314658;
	b11->payment_secret = tal(b11, struct secret);
	memset(b11->payment_secret, 0x11, sizeof(*b11->payment_secret));
	if (!hex_decode("0001020304050607080900010203040506070809000102030405060708090102",
			strlen("0001020304050607080900010203040506070809000102030405060708090102"),
			&b11->payment_hash, sizeof(b11->payment_hash)))
		abort();
	b11->description = "1 cup coffee";
	b11->expiry = 60;
	set_feature_bit(&b11->features, 8);
	set_feature_bit(&b11->features, 14);
	return b11;
}

/* The 52 u5s of a 256-bit `p` or `s` field, as the encoder lays them out. */
static u5 *field256(const tal_t *ctx, const void *src)
{
	u5 *d = tal_arr(ctx, u5, 0);
	bech32_push_bits(&d, src, 256);
	return d;
}

/* Add a second copy of tag `type` (a well-formed 52-u5 field) and decode. */
static struct bolt11 *second_field_then_decode(struct bolt11 *b11, char type,
						const void *data,
						const char **enc,
						const char **fail)
{
	struct bolt11_field *extra = tal(b11, struct bolt11_field);

	extra->tag = type;
	extra->data = field256(extra, data);
	list_add_tail(&b11->extra_fields, &extra->list);

	dev_bolt11_omit_c_value = true;
	*enc = bolt11_encode(tmpctx, b11, false, test_sign, NULL);
	dev_bolt11_omit_c_value = false;
	return bolt11_decode(tmpctx, *enc, NULL, NULL, NULL, fail);
}

int main(int argc, char *argv[])
{
	struct bolt11 *two_p, *same_p, *two_s;
	const char *two_p_str, *same_p_str, *two_s_str;
	const char *two_p_fail, *same_p_fail, *two_s_fail;
	struct sha256 first_hash, other_hash;
	struct secret other_secret;

	common_setup(argv[0]);

	/* BOLT #11:
	 *
	 * NB: all the following examples are signed with `priv_key`=`e126f68f7eafcc8b74f54d269fe206be715000f94dac067d1c04a8ca3b2db734`.
	 */
	if (!hex_decode("e126f68f7eafcc8b74f54d269fe206be715000f94dac067d1c04a8ca3b2db734",
			strlen("e126f68f7eafcc8b74f54d269fe206be715000f94dac067d1c04a8ca3b2db734"),
			&privkey, sizeof(privkey)))
		abort();

	memset(&first_hash, 0, sizeof(first_hash));
	if (!hex_decode("0001020304050607080900010203040506070809000102030405060708090102",
			strlen("0001020304050607080900010203040506070809000102030405060708090102"),
			&first_hash, sizeof(first_hash)))
		abort();
	memset(&other_hash, 0xff, sizeof(other_hash));
	memset(&other_secret, 0x22, sizeof(other_secret));

	/* BOLT #11, "Examples of Invalid Invoices":
	 *
	 * > ### Two distinct p fields in an otherwise valid invoice.
	 */
	two_p = second_field_then_decode(coffee(), 'p', &other_hash,
					 &two_p_str, &two_p_fail);
	if (two_p)
		warnx("two distinct p accepted (first payment_hash kept: %s): %s",
		      memcmp(&two_p->payment_hash, &first_hash,
			     sizeof(first_hash)) == 0 ? "yes" : "no",
		      two_p_str);
	else
		warnx("two distinct p rejected: %s", two_p_fail);

	/* BOLT #11:
	 *
	 * > ### The same p field twice in an otherwise valid invoice.
	 */
	same_p = second_field_then_decode(coffee(), 'p', &first_hash,
					  &same_p_str, &same_p_fail);
	if (same_p)
		warnx("same p twice accepted (first payment_hash kept: %s): %s",
		      memcmp(&same_p->payment_hash, &first_hash,
			     sizeof(first_hash)) == 0 ? "yes" : "no",
		      same_p_str);
	else
		warnx("same p twice rejected: %s", same_p_fail);

	/* BOLT #11:
	 *
	 * > ### Two distinct s fields in an otherwise valid invoice.
	 */
	two_s = second_field_then_decode(coffee(), 's', &other_secret,
					 &two_s_str, &two_s_fail);
	if (two_s)
		warnx("two distinct s accepted (first payment_secret kept: %s): %s",
		      (two_s->payment_secret
		       && memcmp(two_s->payment_secret, "\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11\x11",
				 sizeof(*two_s->payment_secret)) == 0) ? "yes" : "no",
		      two_s_str);
	else
		warnx("two distinct s rejected: %s", two_s_fail);

	/* BOLT #11:
	 *
	 * A reader...
	 * - MUST fail the payment unless exactly one `p` field and exactly one
	 *   `s` field are present.
	 */
	assert(!two_p);
	assert(!same_p);
	assert(!two_s);

	common_shutdown();
}

Build and run it with make unittest/common/test/run-bolt11-duplicate-p-s. The output on master 1ebc4943 is:

run-bolt11-duplicate-p-s: two distinct p accepted (first payment_hash kept: yes): lnbc2500u1pvjluezsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygspp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpu9qrsgqpp5lllllllllllllllllllllllllllllllllllllllllllllllllllskg75fphvn0pgjw6vtsm7nmp76y2nffzgydpl40veyegmve0q3wl5pg5ky9xd4cnx7cuel8jn6v9ysns20vd6sehh0zv0quf56ugl36gp5n4uqy
run-bolt11-duplicate-p-s: same p twice accepted (first payment_hash kept: yes): lnbc2500u1pvjluezsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygspp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpu9qrsgqpp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqsqwpc2yzcv68femngadqjkp2p5xdx20gv79puw5my3jqxh764fz9fxg0ukyl9sn3klfw8aus7gc2sr2q4htfey2y8leyjfyxvttgdysqyz59qd
run-bolt11-duplicate-p-s: two distinct s accepted (first payment_secret kept: yes): lnbc2500u1pvjluezsp5zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zygspp5qqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqqqsyqcyq5rqwzqfqypqdq5xysxxatsyp3k7enxv4jsxqzpu9qrsgqsp5yg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3zyg3q6mwaes2cclntjrk4aggagps5zns7rd70cm56pw2v49lqq60styln50g29ju2xpvqdvv3my00nkhzv70srfkawqa46c9z4aefvupctfcqdfc55d
run-bolt11-duplicate-p-s: common/test/run-bolt11-duplicate-p-s.c:180: main: Assertion `!two_p' failed.
make: *** [Makefile:942: unittest/common/test/run-bolt11-duplicate-p-s] Error 134

bolt11_decode_nosig sends the repeat to unknown_field through the have_field[type] && !decoder->allow_duplicates branch at bolt11.c:924. decode returns the invoice as valid, with the first p as payment_hash and the first s as payment_secret, and the second copy listed under extra.

decode should fail each invoice. BOLT 11 Tagged Fields, Requirements, line 215, says a reader "MUST fail the payment unless exactly one p field and exactly one s field are present."

getinfo output

master 1ebc4943ed37: "version": "1ebc494", "network": "regtest"
v26.06.9, a2176589f235: "version": "a217658", "network": "regtest"