From 0871bf09996ba55bce94e92c4b6c14e028277ab4 Mon Sep 17 00:00:00 2001
From: niftynei <niftynei@gmail.com>
Date: Thu, 22 Oct 2020 12:09:14 -0500
Subject: [PATCH] features: have clear_feature_bit correctly resize bitfield

There's a spec rule about only ever sending a correctly sized
feature-bits, so as a precaution we have `clear_feature_bit` correctly
resize when a bit is cleared.
---
 common/features.c          | 16 ++++++++++++++++
 common/test/run-features.c | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 50 insertions(+)

diff --git a/common/features.c b/common/features.c
index 581799761..c8daf89ea 100644
--- a/common/features.c
+++ b/common/features.c
@@ -121,6 +121,21 @@ static const struct dependency feature_deps[] = {
 #endif
 };
 
+static void trim_features(u8 **features)
+{
+	size_t trim, len = tal_bytelen(*features);
+
+	/* Don't try to tal_resize a NULL array */
+	if (len == 0)
+		return;
+
+	/* Big-endian bitfields are weird, but it means we trim
+	 * from the front: */
+	for (trim = 0; trim < len && (*features)[trim] == 0; trim++);
+	memmove(*features, *features + trim, len - trim);
+	tal_resize(features, len - trim);
+}
+
 static void clear_feature_bit(u8 *features, u32 bit)
 {
 	size_t bytenum = bit / 8, bitnum = bit % 8, len = tal_count(features);
@@ -234,6 +249,7 @@ u8 *get_agreed_channelfeatures(const tal_t *ctx,
 		if (!feature_offered(their_features, i)) {
 			clear_feature_bit(f, COMPULSORY_FEATURE(i));
 			clear_feature_bit(f, OPTIONAL_FEATURE(i));
+			trim_features(&f);
 			continue;
 		}
 		max_len = (i / 8) + 1;
diff --git a/common/test/run-features.c b/common/test/run-features.c
index 9efd9933d..0bbe1195a 100644
--- a/common/test/run-features.c
+++ b/common/test/run-features.c
@@ -179,6 +179,39 @@ static void test_feature_set_or(void)
 	}
 }
 
+static void test_feature_trim(void)
+{
+	struct feature_set *f;
+	for (size_t i = 0; i < ARRAY_SIZE(f->bits); i++) {
+		f = talz(tmpctx, struct feature_set);
+
+		f->bits[i] = tal_arr(f, u8, 0);
+		set_feature_bit(&f->bits[i], 255);
+		assert(tal_bytelen(f->bits[i]) == 32);
+		clear_feature_bit(f->bits[i], 255);
+		trim_features(&f->bits[i]);
+		assert(tal_bytelen(f->bits[i]) == 0);
+
+		set_feature_bit(&f->bits[i], 7);
+		assert(tal_bytelen(f->bits[i]) == 1);
+		set_feature_bit(&f->bits[i], 255);
+		assert(tal_bytelen(f->bits[i]) == 32);
+		clear_feature_bit(f->bits[i], 255);
+		trim_features(&f->bits[i]);
+		assert(tal_bytelen(f->bits[i]) == 1);
+		clear_feature_bit(f->bits[i], 7);
+		trim_features(&f->bits[i]);
+		assert(tal_bytelen(f->bits[i]) == 0);
+
+		set_feature_bit(&f->bits[i], 8);
+		set_feature_bit(&f->bits[i], 10);
+		assert(tal_bytelen(f->bits[i]) == 2);
+		clear_feature_bit(f->bits[i], 10);
+		trim_features(&f->bits[i]);
+		assert(tal_bytelen(f->bits[i]) == 2);
+	}
+}
+
 int main(void)
 {
 	u8 *bits;
@@ -265,6 +298,7 @@ int main(void)
 
 	test_featurebits_or();
 	test_feature_set_or();
+	test_feature_trim();
 
 	wally_cleanup(0);
 	tal_free(tmpctx);