Unverified dual funding tx signatures

channeld/channeld.c

@@ -3852,6 +3852,8 @@ static void resume_splice_negotiation(struct peer *peer,
 	enum tx_role our_role = inflight->i_am_initiator
 						   ? TX_INITIATOR
 						   : TX_ACCEPTER;
+	enum tx_role their_role =
+	    our_role == TX_INITIATOR ? TX_ACCEPTER : TX_INITIATOR;
 	const u8 *wit_script;
 	struct channel_id cid;
 	enum peer_wire type;
@@ -3871,6 +3873,7 @@ static void resume_splice_negotiation(struct peer *peer,
 	const u8 *msg_received;
 	struct witness **inws;
 	struct bitcoin_signature *their_sig;
+	const char *err;
 
 	if (peer->splicing) {
 		inws = peer->splicing->inws;
@@ -4112,12 +4115,6 @@ static void resume_splice_negotiation(struct peer *peer,
 					 splice_funding_index,
 					 wit_script);
 
-		if (tal_count(inws) > current_psbt->num_inputs)
-			peer_failed_warn(peer->pps, &peer->channel_id,
-					 "%zu too many witness elements"
-					 " received",
-					 tal_count(inws) - current_psbt->num_inputs);
-
 		/* We put the PSBT + sigs all together */
 		for (size_t j = 0, i = 0; i < current_psbt->num_inputs; i++) {
 			struct wally_psbt_input *in =
@@ -4152,10 +4149,26 @@ static void resume_splice_negotiation(struct peer *peer,
 						 tal_count(inws));
 			}
 
+			err = check_tx_signatures_witness(tmpctx, in, inws[j]);
+			if (err)
+				peer_failed_warn(
+				    peer->pps, &peer->channel_id,
+				    "Bad witness at position %zu: %s", j, err);
+
 			psbt_finalize_input(current_psbt, in,
 					    inws[j++]);
 		}
 
+		/* Same checks as dualopen. The splice funding input is not
+		 * in the witness array; its signature is the TLV. Extra
+		 * witnesses used to be ignored if they fit under num_inputs. */
+		err = check_tx_signatures_witness_count(
+		    tmpctx, current_psbt, their_role, tal_count(inws),
+		    (int)splice_funding_index);
+		if (err)
+			peer_failed_warn(peer->pps, &peer->channel_id, "%s",
+					 err);
+
 		final_tx = bitcoin_tx_with_psbt(tmpctx, current_psbt);
 
 		wit_stack = bitcoin_witness_2of2(final_tx, &splice_sig,

common/psbt_internal.c

@@ -1,7 +1,9 @@
 #include "config.h"
 #include <assert.h>
 #include <bitcoin/psbt.h>
+#include <bitcoin/pubkey.h>
 #include <bitcoin/script.h>
+#include <ccan/tal/str/str.h>
 #include <common/psbt_internal.h>
 #include <common/psbt_open.h>
 #include <common/utils.h>
@@ -32,6 +34,60 @@ static bool next_size(const u8 **cursor, size_t *max, size_t *size)
 	return true;
 }
 
+/* If @elements is non-NULL it must hold at least as many slots as the
+ * stack claims. A failure may have written a prefix; ignore it. */
+static bool parse_witness_stack(const u8 *witness, size_t *num_items,
+				struct witness_element *elements)
+{
+	const u8 *cursor = witness;
+	size_t max = tal_bytelen(witness);
+	size_t n, item_len;
+
+	if (!next_size(&cursor, &max, &n))
+		return false;
+
+	for (size_t i = 0; i < n; i++) {
+		if (!next_size(&cursor, &max, &item_len))
+			return false;
+		if (max < item_len)
+			return false;
+		if (elements) {
+			elements[i].data = cursor;
+			elements[i].len = item_len;
+		}
+		cursor += item_len;
+		max -= item_len;
+	}
+
+	/* Encoding ends here; anything left is not a witness. */
+	if (max != 0)
+		return false;
+
+	*num_items = n;
+	return true;
+}
+
+bool witness_stack_items(const u8 *witness, size_t *num_items)
+{
+	return parse_witness_stack(witness, num_items, NULL);
+}
+
+const struct witness_element *witness_stack_elements(const tal_t *ctx,
+						     const u8 *witness)
+{
+	size_t n;
+	struct witness_element *elements;
+
+	if (!parse_witness_stack(witness, &n, NULL))
+		return NULL;
+
+	/* Each item costs at least its length byte, so n fits the blob. */
+	elements = tal_arr(ctx, struct witness_element, n);
+	if (!parse_witness_stack(witness, &n, elements))
+		return tal_free(elements);
+	return elements;
+}
+
 static u8 *next_script(const tal_t *ctx, const u8 **cursor, size_t *max)
 {
 	const u8 *p;
@@ -169,3 +225,198 @@ psbt_to_witnesses(const tal_t *ctx,
 
 	return witnesses;
 }
+
+/* Bitcoin Core src/policy/policy.h. Applied only to the type they belong to. */
+#define MAX_STANDARD_P2WSH_STACK_ITEMS 100
+#define MAX_STANDARD_P2WSH_STACK_ITEM_SIZE 80
+#define MAX_STANDARD_P2WSH_SCRIPT_SIZE 3600
+#define MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE 80
+
+/* BIP341 annex tag. BIP342 leaf version is the low 7 bits of 0xc0. */
+#define WITNESS_ANNEX_TAG 0x50
+#define TAPROOT_LEAF_MASK 0xfe
+#define TAPROOT_LEAF_TAPSCRIPT 0xc0
+
+/* A P2WPKH witness is always a signature and a compressed pubkey.
+ * No byte cap: the shape is fixed. */
+static const char *check_p2wpkh_witness(const tal_t *ctx,
+					const struct witness_element *elems)
+{
+	size_t n = tal_count(elems);
+
+	if (n != 2)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "P2WPKH witness has %zu items, "
+			       "expected a signature and a public key",
+			       n);
+	if (elems[1].len != PUBKEY_CMPR_LEN)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "P2WPKH public key is %zu bytes",
+			       elems[1].len);
+	/* Sighash byte sits after the DER signature. No interpreter. */
+	if (elems[0].len == 0 || elems[0].data[elems[0].len - 1] != SIGHASH_ALL)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "P2WPKH signature is not SIGHASH_ALL");
+	return NULL;
+}
+
+/* Items below the witness script, not the script itself. */
+static const char *check_p2wsh_witness(const tal_t *ctx,
+				       const struct witness_element *elems)
+{
+	size_t n = tal_count(elems);
+	size_t stack_items = n - 1;
+
+	if (elems[n - 1].len > MAX_STANDARD_P2WSH_SCRIPT_SIZE)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "witness script is %zu bytes",
+			       elems[n - 1].len);
+
+	if (stack_items > MAX_STANDARD_P2WSH_STACK_ITEMS)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "%zu stack items",
+			       stack_items);
+
+	for (size_t i = 0; i < stack_items; i++) {
+		if (elems[i].len > MAX_STANDARD_P2WSH_STACK_ITEM_SIZE)
+			return tal_fmt(ctx,
+				       "witness is non-standard: "
+				       "stack item %zu is %zu bytes",
+				       i, elems[i].len);
+	}
+	/* FIXME: no SIGHASH_ALL check. Which stack items are signatures
+	 * depends on the witness script, so this needs an interpreter. */
+	return NULL;
+}
+
+/* Key path has no size policy. Tapscript limits the items under the
+ * script; an annex is non-standard. */
+static const char *check_p2tr_witness(const tal_t *ctx,
+				      const struct witness_element *elems)
+{
+	size_t n = tal_count(elems);
+	const struct witness_element *control;
+
+	if (n >= 2 && elems[n - 1].len > 0
+	    && elems[n - 1].data[0] == WITNESS_ANNEX_TAG)
+		return tal_fmt(ctx, "witness is non-standard: taproot annex");
+
+	/* Key path. 64 bytes omits the sighash: that is SIGHASH_DEFAULT,
+	 * which is SIGHASH_ALL. 65 bytes must end in SIGHASH_ALL. */
+	if (n == 1) {
+		const struct witness_element *sig = &elems[0];
+
+		if (sig->len == 64
+		    || (sig->len == 65 && sig->data[64] == SIGHASH_ALL))
+			return NULL;
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "P2TR key path signature is not SIGHASH_ALL");
+	}
+
+	if (n < 2)
+		return NULL;
+
+	/* FIXME: no SIGHASH_ALL check on a script-path spend. Which stack
+	 * items are signatures depends on the leaf script. */
+
+	control = &elems[n - 1];
+	if (control->len == 0)
+		return tal_fmt(ctx,
+			       "witness is non-standard: "
+			       "empty taproot control block");
+	if ((control->data[0] & TAPROOT_LEAF_MASK) != TAPROOT_LEAF_TAPSCRIPT)
+		return NULL;
+
+	for (size_t i = 0; i + 2 < n; i++) {
+		if (elems[i].len > MAX_STANDARD_TAPSCRIPT_STACK_ITEM_SIZE)
+			return tal_fmt(ctx,
+				       "witness is non-standard: "
+				       "tapscript stack item %zu is %zu bytes",
+				       i, elems[i].len);
+	}
+	return NULL;
+}
+
+/* Our inputs carry witness_utxo. Peer inputs carry the prevtx in utxo. */
+static const u8 *spent_scriptpubkey(const struct wally_psbt_input *in,
+				    size_t *script_len)
+{
+	if (in->witness_utxo && in->witness_utxo->script) {
+		*script_len = in->witness_utxo->script_len;
+		return in->witness_utxo->script;
+	}
+	if (in->utxo && in->index < in->utxo->num_outputs
+	    && in->utxo->outputs[in->index].script) {
+		*script_len = in->utxo->outputs[in->index].script_len;
+		return in->utxo->outputs[in->index].script;
+	}
+	*script_len = 0;
+	return NULL;
+}
+
+const char *check_tx_signatures_witness(const tal_t *ctx,
+					const struct wally_psbt_input *in,
+					const struct witness *witness)
+{
+	const struct witness_element *elems;
+	const u8 *script;
+	size_t script_len;
+
+	if (!witness || !witness->witness_data ||
+	    tal_count(witness->witness_data) == 0)
+		return tal_fmt(ctx, "witness is empty");
+
+	elems = witness_stack_elements(ctx, witness->witness_data);
+	if (!elems)
+		return tal_fmt(ctx, "witness is truncated or has trailing bytes");
+
+	if (tal_count(elems) == 0)
+		return tal_fmt(ctx, "witness is empty");
+
+	script = spent_scriptpubkey(in, &script_len);
+	if (script && is_p2wpkh(script, script_len, NULL))
+		return check_p2wpkh_witness(ctx, elems);
+	if (script && is_p2wsh(script, script_len, NULL))
+		return check_p2wsh_witness(ctx, elems);
+	if (script && is_p2tr(script, script_len, NULL))
+		return check_p2tr_witness(ctx, elems);
+
+	return tal_fmt(ctx,
+		       "witness is non-standard: "
+		       "input is not P2WPKH, P2WSH or P2TR");
+}
+
+const char *check_tx_signatures_witness_count(const tal_t *ctx,
+					     const struct wally_psbt *psbt,
+					     enum tx_role their_role,
+					     size_t num_witnesses,
+					     int input_index_to_ignore)
+{
+	size_t peer_inputs = 0;
+
+	for (size_t i = 0; i < psbt->num_inputs; i++) {
+		u64 in_serial;
+
+		if (input_index_to_ignore >= 0
+		    && i == (size_t)input_index_to_ignore)
+			continue;
+		if (!psbt_get_serial_id(&psbt->inputs[i].unknowns, &in_serial))
+			continue;
+		if (in_serial % 2 != their_role)
+			continue;
+		peer_inputs++;
+	}
+
+	if (peer_inputs != num_witnesses)
+		return tal_fmt(ctx,
+			       "Mismatched witness stack count, expecting %zu, "
+			       "got %zu",
+			       peer_inputs, num_witnesses);
+	return NULL;
+}

common/psbt_internal.h

@@ -2,13 +2,30 @@
 #define LIGHTNING_COMMON_PSBT_INTERNAL_H
 
 #include "config.h"
+#include <ccan/short_types/short_types.h>
 #include <ccan/tal/tal.h>
 #include <common/tx_roles.h>
 
 struct wally_psbt;
 struct wally_psbt_input;
 struct witness;
 
+/* One element of a witness stack. @data points into the witness blob. */
+struct witness_element {
+	const u8 *data;
+	size_t len;
+};
+
+/* True iff witness is a complete witness stack: item count, that many
+ * items, no trailing bytes. @num_items is set only on success. */
+bool witness_stack_items(const u8 *witness, size_t *num_items);
+
+/* Elements of a complete witness stack, pointing into @witness.
+ * NULL if truncated or trailing bytes remain. An empty stack is a
+ * zero-length array, not NULL. */
+const struct witness_element *witness_stack_elements(const tal_t *ctx,
+						     const u8 *witness);
+
 /* psbt_finalize_input - Finalize an input with a given witness stack
  *
  * Sets the given witness elements onto the PSBT. Also finalizes
@@ -21,6 +38,22 @@ void psbt_finalize_input(const tal_t *ctx,
 			 struct wally_psbt_input *in,
 			 const struct witness *witness);
 
+/* Minimal tx_signatures witness check, shared by dual open and splice.
+ * NULL if acceptable. */
+const char *check_tx_signatures_witness(const tal_t *ctx,
+					const struct wally_psbt_input *in,
+					const struct witness *witness);
+
+/* The number of witnesses must equal the inputs added by the sending
+ * node. @input_index_to_ignore is not counted (the splice funding
+ * input, whose signature is not in the witness array). Pass -1 to
+ * count every input of @their_role. */
+const char *check_tx_signatures_witness_count(const tal_t *ctx,
+					     const struct wally_psbt *psbt,
+					     enum tx_role their_role,
+					     size_t num_witnesses,
+					     int input_index_to_ignore);
+
 /* psbt_to_witnesses - Take a side's sigs from a PSBT and copy to a
  * 			    wire witness
  *

common/test/Makefile

@@ -11,6 +11,11 @@ COMMON_TEST_COMMON_OBJS :=				\
 	common/utils.o
 
 $(COMMON_TEST_PROGRAMS): $(COMMON_TEST_COMMON_OBJS) $(BITCOIN_OBJS)
+
+# These call into psbt_internal, which is not #included.
+common/test/run-tx_signatures_witness	\
+common/test/run-tx_signatures_count:	\
+	libcommon.a
 $(COMMON_TEST_OBJS): $(COMMON_HEADERS) $(WIRE_HEADERS) $(COMMON_SRC) common/test/Makefile
 
 ALL_C_SOURCES += $(COMMON_TEST_SRC)