validate msgpack record array length before indexing

include/fluent-bit/flb_pack.h

@@ -104,6 +104,8 @@ int flb_pack_to_json_format_type(const char *str);
 int flb_pack_to_json_date_type(const char *str);
 
 void flb_pack_print(const char *data, size_t bytes);
+int flb_metadata_pop_from_msgpack(msgpack_object **metadata, msgpack_unpacked *upk,
+                                  msgpack_object **map);
 int flb_msgpack_to_json(char *json_str, size_t str_len,
                         const msgpack_object *obj,
                         int escape_unicode);

src/flb_pack.c

@@ -839,6 +839,15 @@ int flb_metadata_pop_from_msgpack(msgpack_object **metadata, msgpack_unpacked *u
         return -1;
     }
 
+    if (upk->data.via.array.size < 2) {
+        return -1;
+    }
+
+    if (upk->data.via.array.ptr[0].type != MSGPACK_OBJECT_ARRAY ||
+        upk->data.via.array.ptr[0].via.array.size < 2) {
+        return -1;
+    }
+
     *metadata = &upk->data.via.array.ptr[0].via.array.ptr[1];
     *map = &upk->data.via.array.ptr[1];
 
@@ -852,14 +861,15 @@ static int pack_print_fluent_record(size_t cnt, msgpack_unpacked result)
     msgpack_object  *obj;
     struct flb_time  tms;
     msgpack_object   o;
+    int              ret;
 
     root = result.data;
-    if (root.type != MSGPACK_OBJECT_ARRAY) {
+    if (root.type != MSGPACK_OBJECT_ARRAY || root.via.array.size < 2) {
         return -1;
     }
 
     o = root.via.array.ptr[0];
-    if (o.type != MSGPACK_OBJECT_ARRAY) {
+    if (o.type != MSGPACK_OBJECT_ARRAY || o.via.array.size != 2) {
         return -1;
     }
 
@@ -872,8 +882,15 @@ static int pack_print_fluent_record(size_t cnt, msgpack_unpacked result)
     }
 
     /* This is a Fluent Bit record, just do the proper unpacking/printing */
-    flb_time_pop_from_msgpack(&tms, &result, &obj);
-    flb_metadata_pop_from_msgpack(&metadata, &result, &obj);
+    ret = flb_time_pop_from_msgpack(&tms, &result, &obj);
+    if (ret != 0) {
+        return -1;
+    }
+
+    ret = flb_metadata_pop_from_msgpack(&metadata, &result, &obj);
+    if (ret != 0) {
+        return -1;
+    }
 
     fprintf(stdout, "[%zd] [[%"PRId32".%09lu, ", cnt, (int32_t) tms.tm.tv_sec, tms.tm.tv_nsec);
 

src/flb_time.c

@@ -405,6 +405,10 @@ int flb_time_pop_from_msgpack(struct flb_time *time, msgpack_unpacked *upk,
         return -1;
     }
 
+    if (upk->data.via.array.size < 2) {
+        return -1;
+    }
+
     obj = upk->data.via.array.ptr[0];
 
     if (obj.type == MSGPACK_OBJECT_ARRAY) {

tests/internal/flb_time.c

@@ -466,8 +466,59 @@ void test_iana_zone_to_utc_offset()
     }
 }
 
+/* flb_time_pop_from_msgpack must reject record arrays shorter than 2
+ * elements instead of reading via.array.ptr[0]/ptr[1] out of bounds. */
+static void pop_from_array(const char *data, size_t size, int expect)
+{
+    struct flb_time tm;
+    msgpack_object *map;
+    msgpack_unpacked result;
+    int ret;
+
+    msgpack_unpacked_init(&result);
+    msgpack_unpack_next(&result, data, size, NULL);
+
+    ret = flb_time_pop_from_msgpack(&tm, &result, &map);
+    if (!TEST_CHECK(ret == expect)) {
+        TEST_MSG("got %d, expect %d", ret, expect);
+    }
+
+    msgpack_unpacked_destroy(&result);
+}
+
+void test_pop_from_msgpack_short_array()
+{
+    msgpack_packer mp_pck;
+    msgpack_sbuffer mp_sbuf;
+
+    /* empty array: [] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 0);
+    pop_from_array(mp_sbuf.data, mp_sbuf.size, -1);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+
+    /* single element: [ timestamp ] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 1);
+    msgpack_pack_int(&mp_pck, SEC_32BIT);
+    pop_from_array(mp_sbuf.data, mp_sbuf.size, -1);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+
+    /* valid record: [ timestamp, map ] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 2);
+    msgpack_pack_int(&mp_pck, SEC_32BIT);
+    msgpack_pack_map(&mp_pck, 0);
+    pop_from_array(mp_sbuf.data, mp_sbuf.size, 0);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+}
+
 TEST_LIST = {
     { "flb_time_to_nanosec"           , test_to_nanosec},
+    { "pop_from_msgpack_short_array"  , test_pop_from_msgpack_short_array},
     { "flb_time_append_to_mpack_v1"   , test_append_to_mpack_v1},
     { "msgpack_to_time_int"           , test_msgpack_to_time_int},
     { "msgpack_to_time_double"        , test_msgpack_to_time_double},

tests/internal/pack.c

@@ -1287,6 +1287,71 @@ void test_json_pack_token_count_overflow()
     flb_pack_state_reset(&state);
 }
 
+/* flb_metadata_pop_from_msgpack must reject record arrays that are shorter
+ * than 2 elements or whose first element is a short array, instead of reading
+ * via.array.ptr[1] out of bounds. */
+static void metadata_pop_from_array(const char *data, size_t size, int expect)
+{
+    msgpack_object *metadata;
+    msgpack_object *map;
+    msgpack_unpacked result;
+    int ret;
+
+    msgpack_unpacked_init(&result);
+    msgpack_unpack_next(&result, data, size, NULL);
+
+    ret = flb_metadata_pop_from_msgpack(&metadata, &result, &map);
+    if (!TEST_CHECK(ret == expect)) {
+        TEST_MSG("got %d, expect %d", ret, expect);
+    }
+
+    msgpack_unpacked_destroy(&result);
+}
+
+void test_metadata_pop_from_msgpack_short_array()
+{
+    msgpack_packer mp_pck;
+    msgpack_sbuffer mp_sbuf;
+
+    /* empty array: [] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 0);
+    metadata_pop_from_array(mp_sbuf.data, mp_sbuf.size, -1);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+
+    /* single element: [ [meta, ...] ] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 1);
+    msgpack_pack_array(&mp_pck, 2);
+    msgpack_pack_int(&mp_pck, 0);
+    msgpack_pack_map(&mp_pck, 0);
+    metadata_pop_from_array(mp_sbuf.data, mp_sbuf.size, -1);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+
+    /* first element is a short array: [ [ts], map ] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 2);
+    msgpack_pack_array(&mp_pck, 1);
+    msgpack_pack_int(&mp_pck, 0);
+    msgpack_pack_map(&mp_pck, 0);
+    metadata_pop_from_array(mp_sbuf.data, mp_sbuf.size, -1);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+
+    /* valid record: [ [ts, meta], map ] */
+    msgpack_sbuffer_init(&mp_sbuf);
+    msgpack_packer_init(&mp_pck, &mp_sbuf, msgpack_sbuffer_write);
+    msgpack_pack_array(&mp_pck, 2);
+    msgpack_pack_array(&mp_pck, 2);
+    msgpack_pack_int(&mp_pck, 0);
+    msgpack_pack_map(&mp_pck, 0);
+    msgpack_pack_map(&mp_pck, 0);
+    metadata_pop_from_array(mp_sbuf.data, mp_sbuf.size, 0);
+    msgpack_sbuffer_destroy(&mp_sbuf);
+}
+
 TEST_LIST = {
     /* JSON maps iteration */
     { "json_pack"          , test_json_pack },
@@ -1318,5 +1383,8 @@ TEST_LIST = {
     { "json_pack_surrogate_pairs", test_json_pack_surrogate_pairs},
     { "json_pack_surrogate_pairs_with_replacement", test_json_pack_surrogate_pairs_with_replacement},
     { "json_pack_token_count_overflow", test_json_pack_token_count_overflow},
+
+    /* msgpack record array bounds */
+    { "metadata_pop_from_msgpack_short_array", test_metadata_pop_from_msgpack_short_array},
     { 0 }
 };