From 4a922985604011b68ba99b74bf81e477960603cc Mon Sep 17 00:00:00 2001 From: zanarelli Date: Thu, 30 Jul 2026 21:50:44 -0300 Subject: [PATCH 1/3] in_kubernetes_events: fix OOB reads from non-NUL-terminated msgpack strings record_get_field_uint64() and record_get_field_time() called strtoul()/flb_strptime() directly on msgpack_object.via.str.ptr. msgpack strings are raw, length-prefixed bytes into the decode buffer, not NUL-terminated, so these C-string functions could read past the field's true boundary. record_get_field_ptr()'s strncmp() key match had the same latent issue (a key that is a prefix of fieldname could false-match, and a short key could still be read past its bounds by strncmp with a longer fieldname length). A spec-compliant Kubernetes Event field (e.g. resourceVersion as a digit-only JSON string) placed at the edge of the decode buffer is enough to trigger an out-of-bounds read; confirmed via a guard-page harness that reproduces EXC_BAD_ACCESS inside strtoul_l, called from record_get_field_uint64. This is the same bug class fixed same-day for the sibling out_stackdriver plugin (#12022, backported in #12170), and the same class that produced GHSA-5rjf-prwh-pp7q in this project before. Applies the same fix pattern here: copy the field into a bounded, NUL-terminated stack buffer before parsing, and require an exact length match before the key strncmp. A prior contributor flagged the same underlying issue in #12073, but it was self-closed without a fix landing; the vulnerable code is still present at HEAD. Signed-off-by: zanarelli --- .../in_kubernetes_events/kubernetes_events.c | 48 +++++++++++++++++-- 1 file changed, 44 insertions(+), 4 deletions(-) diff --git a/plugins/in_kubernetes_events/kubernetes_events.c b/plugins/in_kubernetes_events/kubernetes_events.c index 3d996806b54..9c4bd20b16f 100644 --- a/plugins/in_kubernetes_events/kubernetes_events.c +++ b/plugins/in_kubernetes_events/kubernetes_events.c @@ -179,6 +179,7 @@ static int refresh_token_if_needed(struct k8s_events *ctx) static msgpack_object *record_get_field_ptr(msgpack_object *obj, const char *fieldname) { int i; + size_t fieldname_len; msgpack_object *k; msgpack_object *v; @@ -186,13 +187,23 @@ static msgpack_object *record_get_field_ptr(msgpack_object *obj, const char *fie return NULL; } + fieldname_len = strlen(fieldname); + for (i = 0; i < obj->via.map.size; i++) { k = &obj->via.map.ptr[i].key; if (k->type != MSGPACK_OBJECT_STR) { continue; } - if (strncmp(k->via.str.ptr, fieldname, strlen(fieldname)) == 0) { + /* + * msgpack strings are not NUL terminated: k->via.str.ptr points + * directly into the decode buffer for exactly k->via.str.size + * bytes. Require an exact length match before comparing so we + * never read past that boundary, and so a key that merely shares + * a prefix with fieldname cannot match. + */ + if ((size_t) k->via.str.size == fieldname_len && + strncmp(k->via.str.ptr, fieldname, fieldname_len) == 0) { v = &obj->via.map.ptr[i].val; return v; } @@ -220,6 +231,7 @@ static int record_get_field_time(msgpack_object *obj, const char *fieldname, str { msgpack_object *v; struct flb_tm tm = { 0 }; + char buf[64]; v = record_get_field_ptr(obj, fieldname); if (v == NULL) { @@ -229,7 +241,19 @@ static int record_get_field_time(msgpack_object *obj, const char *fieldname, str return -1; } - if (flb_strptime(v->via.str.ptr, "%Y-%m-%dT%H:%M:%SZ", &tm) == NULL) { + /* + * msgpack strings are not NUL terminated: v->via.str.ptr points + * directly into the decode buffer for exactly v->via.str.size bytes. + * Copy it into a bounded, NUL-terminated stack buffer before handing + * it to flb_strptime(), instead of scanning the raw buffer directly. + */ + if (v->via.str.size == 0 || v->via.str.size >= sizeof(buf)) { + return -2; + } + memcpy(buf, v->via.str.ptr, v->via.str.size); + buf[v->via.str.size] = '\0'; + + if (flb_strptime(buf, "%Y-%m-%dT%H:%M:%SZ", &tm) == NULL) { return -2; } @@ -242,7 +266,9 @@ static int record_get_field_time(msgpack_object *obj, const char *fieldname, str static int record_get_field_uint64(msgpack_object *obj, const char *fieldname, uint64_t *val) { msgpack_object *v; + char buf[32]; char *end; + size_t len; v = record_get_field_ptr(obj, fieldname); if (v == NULL) { @@ -251,8 +277,22 @@ static int record_get_field_uint64(msgpack_object *obj, const char *fieldname, u /* attempt to parse string as number... */ if (v->type == MSGPACK_OBJECT_STR) { - *val = strtoul(v->via.str.ptr, &end, 10); - if (end == NULL || (end < v->via.str.ptr + v->via.str.size)) { + /* + * msgpack strings are not NUL terminated: v->via.str.ptr points + * directly into the decode buffer for exactly v->via.str.size + * bytes. Copy it into a bounded, NUL-terminated stack buffer + * before calling strtoul() on it, instead of scanning the raw + * buffer directly (no valid uint64 needs more than 20 digits). + */ + len = v->via.str.size; + if (len == 0 || len > sizeof(buf) - 1) { + return -1; + } + memcpy(buf, v->via.str.ptr, len); + buf[len] = '\0'; + + *val = strtoul(buf, &end, 10); + if (end == NULL || end == buf || *end != '\0') { return -1; } return 0; From 3897b6e4d89a709ad794fd56e0417794a67a7089 Mon Sep 17 00:00:00 2001 From: zanarelli Date: Fri, 31 Jul 2026 06:27:49 -0300 Subject: [PATCH 2/3] in_kubernetes_events: tighten timestamp and uint64 parsing validation Require flb_strptime to consume the full copied buffer, treat only ret==0 as a successful timestamp in item_get_timestamp(), and parse uint64 strings with strtoull+errno so malformed or overflowing values fall through to the next timestamp field instead of being accepted. Signed-off-by: Raphael Zanarelli Signed-off-by: zanarelli --- .../in_kubernetes_events/kubernetes_events.c | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/plugins/in_kubernetes_events/kubernetes_events.c b/plugins/in_kubernetes_events/kubernetes_events.c index 9c4bd20b16f..c6b874397b8 100644 --- a/plugins/in_kubernetes_events/kubernetes_events.c +++ b/plugins/in_kubernetes_events/kubernetes_events.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include @@ -253,8 +254,13 @@ static int record_get_field_time(msgpack_object *obj, const char *fieldname, str memcpy(buf, v->via.str.ptr, v->via.str.size); buf[v->via.str.size] = '\0'; - if (flb_strptime(buf, "%Y-%m-%dT%H:%M:%SZ", &tm) == NULL) { - return -2; + { + char *end; + + end = flb_strptime(buf, "%Y-%m-%dT%H:%M:%SZ", &tm); + if (end == NULL || *end != '\0') { + return -2; + } } val->tm.tv_sec = flb_parser_tm2time(&tm, FLB_FALSE); @@ -291,8 +297,9 @@ static int record_get_field_uint64(msgpack_object *obj, const char *fieldname, u memcpy(buf, v->via.str.ptr, len); buf[len] = '\0'; - *val = strtoul(buf, &end, 10); - if (end == NULL || end == buf || *end != '\0') { + errno = 0; + *val = strtoull(buf, &end, 10); + if (errno == ERANGE || end == buf || *end != '\0') { return -1; } return 0; @@ -302,8 +309,7 @@ static int record_get_field_uint64(msgpack_object *obj, const char *fieldname, u return 0; } if (v->type == MSGPACK_OBJECT_NEGATIVE_INTEGER) { - *val = (uint64_t)v->via.i64; - return 0; + return -1; } return -1; } @@ -317,12 +323,12 @@ static int item_get_timestamp(msgpack_object *obj, struct flb_time *event_time) * NULL while having metadata.creationTimestamp set. */ ret = record_get_field_time(obj, "lastTimestamp", event_time); - if (ret != -1) { + if (ret == 0) { return FLB_TRUE; } ret = record_get_field_time(obj, "firstTimestamp", event_time); - if (ret != -1) { + if (ret == 0) { return FLB_TRUE; } @@ -332,7 +338,7 @@ static int item_get_timestamp(msgpack_object *obj, struct flb_time *event_time) } ret = record_get_field_time(metadata, "creationTimestamp", event_time); - if (ret != -1) { + if (ret == 0) { return FLB_TRUE; } From 10e922c8a04bdcfff2ac398640d8cab53cb0429a Mon Sep 17 00:00:00 2001 From: zanarelli Date: Fri, 31 Jul 2026 12:53:38 -0300 Subject: [PATCH 3/3] in_kubernetes_events: reject signed uint64 strings in record_get_field_uint64 strtoull() itself accepts a leading '+'/'-' and skips leading whitespace, so a resourceVersion string like "-5" silently wrapped around into 18446744073709551611 instead of being rejected. Kubernetes always serializes resourceVersion as a plain unsigned digits-only decimal string, so require the first byte to be a digit before calling strtoull(), on top of the existing errno/ERANGE and full-consumption checks. Confirmed via a standalone guard-page harness (same shape as the existing OOB reproduction in this PR): "-5" and "+5" are now rejected (previously accepted, wrapping "-5" to UINT64_MAX-4), a valid digits-only value still round-trips correctly, and overflow beyond UINT64_MAX still correctly fails via the existing ERANGE check. Addresses a CodeRabbit review comment on this PR. Signed-off-by: Raphael Zanarelli Signed-off-by: zanarelli --- plugins/in_kubernetes_events/kubernetes_events.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/plugins/in_kubernetes_events/kubernetes_events.c b/plugins/in_kubernetes_events/kubernetes_events.c index c6b874397b8..9ac2e5e0047 100644 --- a/plugins/in_kubernetes_events/kubernetes_events.c +++ b/plugins/in_kubernetes_events/kubernetes_events.c @@ -22,6 +22,7 @@ #include #include #include +#include #include #include @@ -297,6 +298,18 @@ static int record_get_field_uint64(msgpack_object *obj, const char *fieldname, u memcpy(buf, v->via.str.ptr, len); buf[len] = '\0'; + /* + * strtoull() itself accepts a leading '+'/'-' and skips leading + * whitespace, which would let a value like "-5" silently wrap + * around into a huge positive number instead of being rejected. + * Kubernetes resourceVersion (the only caller) is always a plain, + * unsigned, digits-only decimal string, so require that directly + * before parsing. + */ + if (!isdigit((unsigned char) buf[0])) { + return -1; + } + errno = 0; *val = strtoull(buf, &end, 10); if (errno == ERANGE || end == buf || *end != '\0') {