From: Tobias Brunner <tobias@strongswan.org>
Date: Wed, 19 Nov 2025 09:30:39 +0000 (+0100)
Subject: vici: Enforce maximum length for names when building a message
X-Git-Tag: 6.0.4rc1~4^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9b0b1cb24eec31bd88021236817acd611246ea15;p=thirdparty%2Fstrongswan.git

vici: Enforce maximum length for names when building a message

Otherwise, an integer overflow will shorten the name and cause
unpredictable outcomes.
---

diff --git a/src/libcharon/plugins/vici/suites/test_message.c b/src/libcharon/plugins/vici/suites/test_message.c
index 3bfef1e408..0515fce514 100644
--- a/src/libcharon/plugins/vici/suites/test_message.c
+++ b/src/libcharon/plugins/vici/suites/test_message.c
@@ -284,6 +284,24 @@ START_TEST(test_builder)
 }
 END_TEST
 
+START_TEST(test_builder_limits)
+{
+	vici_builder_t *b;
+
+	b = vici_builder_create();
+	b->add(b, VICI_SECTION_START, "this-section-name-is-too-long-to-be-encoded-this-section-name-is-too-long-to-be-encoded-this-section-name-is-too-long-to-be-encoded-this-section-name-is-too-long-to-be-encoded-this-section-name-is-too-long-to-be-encoded-this-section-name-is-too-long-to-be-encoded");
+	b->add(b,  VICI_KEY_VALUE, "key1", chunk_from_str("value1"));
+	b->add(b, VICI_SECTION_END);
+	ck_assert(!b->finalize(b));
+
+	b = vici_builder_create();
+	b->add(b, VICI_SECTION_START, "section1");
+	b->add(b,  VICI_KEY_VALUE, "this-key-value-name-is-too-long-to-be-encoded-this-key-value-name-is-too-long-to-be-encoded-this-key-value-name-is-too-long-to-be-encoded-this-key-value-name-is-too-long-to-be-encoded-this-key-value-name-is-too-long-to-be-encoded-this-key-value-name-is-too-long-to-be-encoded", chunk_from_str("value1"));
+	b->add(b, VICI_SECTION_END);
+	ck_assert(!b->finalize(b));
+}
+END_TEST
+
 START_TEST(test_builder_fmt)
 {
 	enumerator_t *parse, *tmpl;
@@ -426,6 +444,7 @@ Suite *message_suite_create()
 
 	tc = tcase_create("builder encode");
 	tcase_add_test(tc, test_builder);
+	tcase_add_test(tc, test_builder_limits);
 	suite_add_tcase(s, tc);
 
 	tc = tcase_create("builder format encode");
diff --git a/src/libcharon/plugins/vici/vici_builder.c b/src/libcharon/plugins/vici/vici_builder.c
index 3703d909a9..b9017c84b3 100644
--- a/src/libcharon/plugins/vici/vici_builder.c
+++ b/src/libcharon/plugins/vici/vici_builder.c
@@ -55,8 +55,7 @@ METHOD(vici_builder_t, add, void,
 	private_vici_builder_t *this, vici_type_t type, ...)
 {
 	va_list args;
-	char *name = NULL;
-	chunk_t value = chunk_empty;
+	chunk_t name = chunk_empty, value = chunk_empty;
 
 	va_start(args, type);
 	switch (type)
@@ -67,10 +66,10 @@ METHOD(vici_builder_t, add, void,
 			break;
 		case VICI_LIST_START:
 		case VICI_SECTION_START:
-			name = va_arg(args, char*);
+			name = chunk_from_str(va_arg(args, char*));
 			break;
 		case VICI_KEY_VALUE:
-			name = va_arg(args, char*);
+			name = chunk_from_str(va_arg(args, char*));
 			value = va_arg(args, chunk_t);
 			break;
 		case VICI_LIST_ITEM:
@@ -83,6 +82,13 @@ METHOD(vici_builder_t, add, void,
 	}
 	va_end(args);
 
+	if (name.len > 0xff)
+	{
+		DBG1(DBG_ENC, "vici name exceeds size limit (%zu > %u)",
+			 name.len, 0xff);
+		this->error++;
+		return;
+	}
 	if (value.len > 0xffff)
 	{
 		DBG1(DBG_ENC, "vici value exceeds size limit (%zu > %u)",
@@ -102,18 +108,18 @@ METHOD(vici_builder_t, add, void,
 	switch (type)
 	{
 		case VICI_SECTION_START:
-			this->writer->write_data8(this->writer, chunk_from_str(name));
+			this->writer->write_data8(this->writer, name);
 			this->section++;
 			break;
 		case VICI_SECTION_END:
 			this->section--;
 			break;
 		case VICI_KEY_VALUE:
-			this->writer->write_data8(this->writer, chunk_from_str(name));
+			this->writer->write_data8(this->writer, name);
 			this->writer->write_data16(this->writer, value);
 			break;
 		case VICI_LIST_START:
-			this->writer->write_data8(this->writer, chunk_from_str(name));
+			this->writer->write_data8(this->writer, name);
 			this->list = TRUE;
 			break;
 		case VICI_LIST_ITEM: