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 3bfef1e408922df062db8517b8a52515bab21ca8..0515fce514a6f64c343c98e1cc4652b8f35855ef 100644 (file)
--- 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 3703d909a96e9b05a9379b0e4484e3cc8a48b3a0..b9017c84b3bac669c5c14f1cbf7da281414f408e 100644 (file)
--- 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: