common: handle taken() extra_tlvs in new_existing_htlc properly.

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

Author: rustyrussell

common/htlc_wire.c

@@ -25,6 +25,23 @@ static struct failed_htlc *failed_htlc_dup(const tal_t *ctx,
 	return newf;
 }
 
+/* Helper to duplicate an array of tlv_field (vs an array of tlv_field *) */
+struct tlv_field *tlv_field_arr_dup(const tal_t *ctx,
+				    const struct tlv_field *arr TAKES)
+{
+	struct tlv_field *ret;
+	bool needs_copy = !is_taken(arr);
+
+	ret = tal_dup_talarr(ctx, struct tlv_field, arr);
+	if (needs_copy) {
+		for (size_t i = 0; i < tal_count(ret); i++) {
+			/* We need to attach the value to the correct parent */
+			ret[i].value = tal_dup_talarr(ret, u8, ret[i].value);
+		}
+	}
+	return ret;
+}
+
 struct existing_htlc *new_existing_htlc(const tal_t *ctx,
 					u64 id,
 					enum htlc_state state,
@@ -53,17 +70,10 @@ struct existing_htlc *new_existing_htlc(const tal_t *ctx,
 		existing->failed = failed_htlc_dup(existing, failed);
 	else
 		existing->failed = NULL;
-	if (extra_tlvs) {
-		existing->extra_tlvs = tal_dup_talarr(existing, struct tlv_field, extra_tlvs);
-		for (size_t i = 0; i < tal_count(extra_tlvs); i++) {
-			/* We need to attach the value to the correct parent */
-			existing->extra_tlvs[i].value
-				= tal_dup_talarr(existing, u8,
-						 existing->extra_tlvs[i].value);
-		}
-	} else {
+	if (extra_tlvs)
+		existing->extra_tlvs = tlv_field_arr_dup(existing, extra_tlvs);
+	else
 		existing->extra_tlvs = NULL;
-	}
 
 	return existing;
 }

common/htlc_wire.h

@@ -62,6 +62,10 @@ struct changed_htlc {
 	u64 id;
 };
 
+/* Helper to duplicate an array of tlv_field (vs an array of tlv_field *) */
+struct tlv_field *tlv_field_arr_dup(const tal_t *ctx,
+				    const struct tlv_field *arr TAKES);
+
 struct existing_htlc *new_existing_htlc(const tal_t *ctx,
 					u64 id,
 					enum htlc_state state,

lightningd/htlc_end.c

@@ -3,6 +3,7 @@
 #include <ccan/crypto/siphash24/siphash24.h>
 #include <ccan/tal/str/str.h>
 #include <common/htlc.h>
+#include <common/htlc_wire.h>
 #include <common/pseudorand.h>
 #include <lightningd/htlc_end.h>
 #include <lightningd/log.h>
@@ -148,15 +149,10 @@ struct htlc_in *new_htlc_in(const tal_t *ctx,
 	hin->path_key = tal_dup_or_null(hin, struct pubkey, path_key);
 	memcpy(hin->onion_routing_packet, onion_routing_packet,
 	       sizeof(hin->onion_routing_packet));
-	if (extra_tlvs) {
-		hin->extra_tlvs = tal_dup_talarr(hin, struct tlv_field, extra_tlvs);
-		for (size_t i = 0; i < tal_count(extra_tlvs); i++) {
-			/* We need to attach the value to the correct parent */
-			hin->extra_tlvs[i].value = tal_dup_talarr(hin, u8, hin->extra_tlvs[i].value);
-		}
-	} else {
+	if (extra_tlvs)
+		hin->extra_tlvs = tlv_field_arr_dup(hin, extra_tlvs);
+	else
 		hin->extra_tlvs = NULL;
-	}
 
 	hin->hstate = RCVD_ADD_COMMIT;
 	hin->badonion = 0;
@@ -304,15 +300,10 @@ struct htlc_out *new_htlc_out(const tal_t *ctx,
 
 	hout->path_key = tal_dup_or_null(hout, struct pubkey, path_key);
 
-	if (extra_tlvs) {
-		hout->extra_tlvs = tal_dup_talarr(hout, struct tlv_field, extra_tlvs);
-		for (size_t i = 0; i < tal_count(extra_tlvs); i++) {
-			/* We need to attach the value to the correct parent */
-			hout->extra_tlvs[i].value = tal_dup_talarr(hout, u8, hout->extra_tlvs[i].value);
-		}
-	} else {
+	if (extra_tlvs)
+		hout->extra_tlvs = tlv_field_arr_dup(hout, extra_tlvs);
+	else
 		hout->extra_tlvs = NULL;
-	}
 
 	hout->am_origin = am_origin;
 	if (am_origin) {