in_kubernetes_events: fix OOB reads from non-NUL-terminated msgpack strings

plugins/in_kubernetes_events/kubernetes_events.c

@@ -21,6 +21,8 @@
 #include <sys/types.h>
 #include <sys/stat.h>
 #include <inttypes.h>
+#include <errno.h>
+#include <ctype.h>
 
 #include <fluent-bit/flb_input_plugin.h>
 #include <fluent-bit/flb_network.h>
@@ -179,20 +181,31 @@ 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;
 
     if (obj->type != MSGPACK_OBJECT_MAP) {
         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 +233,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,9 +243,26 @@ 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';
+
+    {
+        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);
     val->tm.tv_nsec = 0;
@@ -242,7 +273,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 +284,35 @@ 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';
+
+        /*
+         * 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') {
             return -1;
         }
         return 0;
@@ -262,8 +322,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;
 }
@@ -277,12 +336,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;
     }
 
@@ -292,7 +351,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;
     }