Skip to content

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

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
zanarellidev wants to merge 1 commit into
base: master
Choose a base branch
from
+44 −4
Open

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

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
48 changes: 44 additions & 4 deletions plugins/in_kubernetes_events/kubernetes_events.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,20 +179,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;
}
Expand Down Expand Up @@ -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) {
Expand All @@ -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;
Comment on lines +250 to +251

@coderabbitai coderabbitai Bot Jul 31, 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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Propagate malformed timestamps as failures.

record_get_field_time() returns -2 for empty or oversized values, but item_get_timestamp() treats any result other than -1 as success at Line 320, Line 325, and Line 335. An invalid lastTimestamp can therefore suppress fallback to firstTimestamp or metadata.creationTimestamp. Accept only ret == 0 downstream, or normalize parse failures to the caller’s failure sentinel.

🤖 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 `@plugins/in_kubernetes_events/kubernetes_events.c` around lines 250 - 251,
Update item_get_timestamp() to treat record_get_field_time() results other than
0 as failures, including -2 for empty or oversized timestamp values. Ensure each
lastTimestamp, firstTimestamp, and metadata.creationTimestamp path only returns
success when ret == 0, allowing fallback timestamps to be attempted after
malformed values.

}
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) {

@coderabbitai coderabbitai Bot Jul 31, 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.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/sh
set -eu

rg -n -C 8 '\bflb_strptime\s*\(' .
rg -n -C 6 'record_get_field_time|item_get_timestamp' \
  plugins/in_kubernetes_events tests/runtime/in_kubernetes_events.c

Repository: fluent/fluent-bit

Length of output: 38889

🏁 Script executed:

#!/bin/sh
set -eu

echo "== kubernetes_events.c parse function =="
sed -n '220,270p' plugins/in_kubernetes_events/kubernetes_events.c

echo "== flb_strptime return usage in flb_parser.c =="
sed -n '1985,2022p' src/flb_parser.c

echo "== kubernetes_events runtime tests mentioning timestamps =="
rg -n -C 4 'timestamp|creationTimestamp|lastTimestamp|firstTimestamp|20.*T[0-9]|invalid' tests/runtime/in_kubernetes_events.c plugins/in_kubernetes_tests || true

echo "== deterministic model of current behavior vs proposed end check =="
python3 - <<'PY'
from datetime import datetime
samples = [
    ("2023-01-15T12:34:56Z", True),
    ("2023-01-15T12:34:56Z extra", False),
    ("", False),
]
for s, exact_ok in samples:
    prefix = "2023-01-15T12:34:56Z"
    # Mimic: parsing succeeds if prefix matches ISO timestamp pattern exactly
    # current NULL-only check accepts valid prefix.
    parsed_ok = s.startswith(prefix)
    end_consumed = len(s) == len(prefix)
    print(f"{s!r}: parsed_ok={parsed_ok}, end_at_null={end_consumed}, current_accepts={parsed_ok}, proposed_accepts={parsed_ok and end_consumed}")
PY

Repository: fluent/fluent-bit

Length of output: 6433

Require flb_strptime() to consume the entire timestamp buffer.

The NULL check accepts valid ISO timestamp prefixes with trailing content, so lastTimestamp, firstTimestamp, or metadata.creationTimestamp could be parsed even when the string contains malformed/garbage suffixes. Capture the return pointer and require *end == '\0'.

🤖 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 `@plugins/in_kubernetes_events/kubernetes_events.c` at line 256, Update the
timestamp parsing around flb_strptime() to capture its returned end pointer and
require both successful parsing and *end == '\0'. Apply this validation
consistently to lastTimestamp, firstTimestamp, and metadata.creationTimestamp so
trailing characters are rejected.

return -2;
}

Expand All @@ -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) {
Expand All @@ -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') {
Comment on lines +287 to +295

@coderabbitai coderabbitai Bot Jul 31, 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.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/sh
set -eu

rg -n -C 8 \
  '\b(record_get_field_uint64|strtoul|strtoull|strtoumax|ERANGE)\b' .

Repository: fluent/fluent-bit

Length of output: 50374

🌐 Web query:

C strtoul overflow ERANGE unsigned long documentation

💡 Result:

When using the C standard library function strtoul, an overflow occurs if the converted value is outside the range of representable values for an unsigned long [1][2]. When this range error occurs: 1. The function returns ULONG_MAX [3][4][5]. 2. The global variable errno is set to ERANGE [3][1][2]. Because ULONG_MAX is a valid return value for successful conversions, you cannot rely solely on the return value to detect an error [1][5]. To correctly check for overflow: 1. Set errno to 0 before calling strtoul [1][2][5]. 2. Perform the conversion [1][2]. 3. Check if errno is equal to ERANGE after the call [3][4][5]. If the input string begins with a minus sign, strtoul performs the conversion as if the value were positive and then negates the result within the unsigned type [4][5]. However, if the absolute value (the non-negated value) is so large that it would overflow even before negation, the function will still trigger the ERANGE error and return ULONG_MAX [6][5].

Citations:

🏁 Script executed:

#!/bin/sh
set -eu

printf '--- kubernetes_events.c relevant function ---\n'
sed -n '266,306p' plugins/in_kubernetes_events/kubernetes_events.c | cat -n

printf '\n--- behavioral probe if compiler is available ---\n'
if command -v cc >/dev/null 2>&1; then
  tmpdir="$(mktemp -d)"
  cat > "$tmpdir/strtoul_probe.c" <<'C'
`#define` _GNU_SOURCE
`#include` <stdio.h>
`#include` <stdlib.h>
`#include` <string.h>
`#include` <errno.h>
`#include` <limits.h>

int main(void) {
  char buf[32];
  char *end;
  unsigned long ul;
  int tests;

  tests = snprintf(buf, sizeof(buf), "%lu", ULONG_MAX);
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul(buf, &end, 10);
  printf("test_unchanged_buffer_returned_ULONG_MAX=%u\n", ul == ULONG_MAX);
  printf("test_unchanged_buffer_errno_ERANGE=%u\n", (int)(errno == ERANGE));
  printf("test_unchanged_buffer_consumed_chars=%ld\n", (long)(end - buf));

  tests = snprintf(buf, sizeof(buf), "%lu", ULONG_MAX + 1ULL);
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul(buf, &end, 10);
  printf("test_oversized_returned_ULONG_MAX=%u\n", ul == ULONG_MAX);
  printf("test_oversized_errno_ERANGE=%u\n", (int)(errno == ERANGE));
  printf("test_oversized_consumed_chars=%ld\n", (long)(end - buf));

  memset(buf, 0, sizeof(buf));
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul("-1", &end, 10);
  printf("test_minus_one_ret=%08lx end=%ld\n", ul, (long)(end - "-1"));
  return 0;
}
C
  if cc -O2 -o "$tmpdir/strtoul_probe" "$tmpdir/strtoul_probe.c" 2>/tmp/strtoul_compile.log; then
    "$tmpdir/strtoul_probe"
  else
    printf 'compile_failed\n'
    cat /tmp/strtoul_compile.log
  fi
  rm -rf "$tmpdir"
else
  printf 'cc not available\n'
fi

Repository: fluent/fluent-bit

Length of output: 246

🏁 Script executed:

#!/bin/sh
set -eu

printf '%s\n' '--- kubernetes_events.c relevant function ---'
sed -n '266,306p' plugins/in_kubernetes_events/kubernetes_events.c | cat -n

printf '%s\n' ''
printf '%s\n' '--- behavioral probe if compiler is available ---'
if command -v cc >/dev/null 2>&1; then
  tmpdir="$(mktemp -d)"
  cat > "$tmpdir/strtoul_probe.c" <<'C'
`#define` _GNU_SOURCE
`#include` <stdio.h>
`#include` <stdlib.h>
`#include` <string.h>
`#include` <errno.h>
`#include` <limits.h>

int main(void) {
  char buf[32];
  char *end;
  unsigned long ul;
  int tests;

  tests = snprintf(buf, sizeof(buf), "%lu", ULONG_MAX);
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul(buf, &end, 10);
  printf("test_unchanged_buffer_returned_ULONG_MAX=%u\n", ul == ULONG_MAX);
  printf("test_unchanged_buffer_errno_ERANGE=%u\n", (int)(errno == ERANGE));
  printf("test_unchanged_buffer_consumed_chars=%ld\n", (long)(end - buf));

  tests = snprintf(buf, sizeof(buf), "%lu", ULONG_MAX + 1ULL);
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul(buf, &end, 10);
  printf("test_oversized_returned_ULONG_MAX=%u\n", ul == ULONG_MAX);
  printf("test_oversized_errno_ERANGE=%u\n", (int)(errno == ERANGE));
  printf("test_oversized_consumed_chars=%ld\n", (long)(end - buf));

  memset(buf, 0, sizeof(buf));
  ul = (unsigned long) 0xdeadbeefdeafbeef; errno = 0; end = buf + strlen(buf) + 1;
  ul = strtoul("-1", &end, 10);
  printf("test_minus_one_ret=%08lx end=%ld\n", ul, (long)(end - "-1"));
  return 0;
}
C
  if cc -O2 -o "$tmpdir/strtoul_probe" "$tmpdir/strtoul_probe.c" 2>/tmp/strtoul_compile.log; then
    "$tmpdir/strtoul_probe"
  else
    printf '%s\n' 'compile_failed'
    cat /tmp/strtoul_compile.log
  fi
  rm -rf "$tmpdir"
else
  printf '%s\n' 'cc not available'
fi

Repository: fluent/fluent-bit

Length of output: 1834

🌐 Web query:

C strtoull base 10 optional sign documentation ERANGE

💡 Result:

The C standard library function strtoull (and its related functions like strtoul) performs integer conversion on a string and is defined to handle optional signs and potential range errors as follows: Optional Signs The input string may begin with an arbitrary amount of whitespace (as determined by isspace) followed by an optional plus (+) or minus (-) sign [1][2]. This applies regardless of the specified base (including base 10) [3][4]. If a minus sign is present, the resulting value is negated [3][5][4]. Since the return type of strtoull is unsigned, this negation follows the rules of unsigned integer arithmetic (often referred to as modular arithmetic or unsigned wraparound) [6]. Range Errors (ERANGE) If the correct value of the conversion falls outside the range of representable values for the unsigned long long type, the function returns ULLONG_MAX and sets errno to ERANGE [1][7]. Usage Note Because ULLONG_MAX is a valid return value on success (it is the maximum representable value), a program should reset errno to 0 before calling strtoull and check it after the call to reliably distinguish between a successful conversion to ULLONG_MAX and a range error [7][8]. Summary of Behavior 1. Whitespace is skipped [1][4]. 2. An optional sign (+ or -) is parsed [3][5]. 3. The remaining characters are converted based on the provided base (or auto-detected if base is 0) [1][2]. 4. If the final value is out of range, ULLONG_MAX is returned and ERANGE is set [1][8]. Top Results: [1], [2], [5], [7], [8]

Citations:

Reject signed or overflowing numeric fields.

record_get_field_uint64() accepts 31-character strings, stores the result of strtoul() in a uint64_t, and never checks errno == ERANGE. This allows out-of-range values to saturate to ULONG_MAX and signs like -1 to be accepted. Use an explicit unsigned parser that accepts digits only, clears/checks errno, and rejects values larger than UINT64_MAX.

🤖 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 `@plugins/in_kubernetes_events/kubernetes_events.c` around lines 287 - 295,
Update record_get_field_uint64() to parse the copied string with an explicit
unsigned, digits-only conversion instead of accepting strtoul’s signed or
overflowing results. Clear errno before conversion, reject any non-digit input
including signs, reject ERANGE and values exceeding UINT64_MAX, and preserve the
existing invalid-field return behavior.

return -1;
}
return 0;
Expand Down
Loading