Skip to content

validate msgpack record array length before indexing #11973

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
saddamr3e wants to merge 3 commits into
base: master
Choose a base branch
from
Open

validate msgpack record array length before indexing #11973

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8d15cd3
time: bounds-check record array in flb_time_pop_from_msgpack
saddamr3e Jun 21, 2026
7bf83d8
pack: bounds-check record array in msgpack pop helpers
saddamr3e Jun 21, 2026
91f2c19
tests: cover short record arrays in flb_time_pop_from_msgpack
saddamr3e Jun 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 11 additions & 2 deletions src/flb_pack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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];

Expand All @@ -854,12 +863,12 @@ static int pack_print_fluent_record(size_t cnt, msgpack_unpacked result)
msgpack_object o;

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;
Comment on lines +866 to 872

@coderabbitai coderabbitai Bot Jun 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Gate record printing on parse success to avoid using invalid decode state.

The new shape checks are good, but they don’t guarantee timestamp decode success for all allowed types (notably malformed EXT payloads). If decode fails and output still proceeds, tms can be consumed in an invalid/uninitialized state. Please check pop helper return values and abort early on failure.

Proposed fix 
 static int pack_print_fluent_record(size_t cnt, msgpack_unpacked result)
 {
     msgpack_object  *metadata;
     msgpack_object   root;
     msgpack_object  *obj;
     struct flb_time  tms;
     msgpack_object   o;
+    int              ret;
@@
-    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;
+    }
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/flb_pack.c` around lines 866 - 872, The shape validation checks for
msgpack objects are in place, but they do not guarantee successful decoding of
the timestamp data, particularly for malformed EXT payloads. The `tms` variable
can remain uninitialized if a pop helper function fails during timestamp
decoding. Add return value checks for the pop helper functions that decode the
timestamp from the array elements validated by the shape checks shown in the
diff, and return early with an error code if any pop operation fails before
attempting to use the decoded `tms` value in subsequent processing.

}

Expand Down
4 changes: 4 additions & 0 deletions src/flb_time.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
51 changes: 51 additions & 0 deletions tests/internal/flb_time.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down
Loading