out_stackdriver: fix payload parsing over-reads and null safety [Backport to 4.2] - #12170
Merged
cosmo0920 merged 3 commits intoJul 30, 2026
Merged
Conversation
Signed-off-by: Yu Yi <yiyu@google.com> (cherry picked from commit 88afc49)
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
try_assign_subfield_int() used atoll() on the copied payload string, which
silently accepts partially-parsed input ("123abc" -> 123), treats an empty
string as 0, and would parse a truncated value from an oversized string.
Numeric httpRequest fields and sourceLocation.line could therefore be set
from malformed input.
Validate the full string with strtoll() instead: reject empty or oversized
strings up front, and only assign when the entire buffer parsed as one
complete integer (endptr at NUL) without overflow (errno). On invalid input
the field is left at its previous value.
Signed-off-by: Yu Yi <yiyu@google.com>
(cherry picked from commit db8c436)
Add a runtime test (sourceLocation_line_invalid_string) asserting that a "123abc" sourceLocation.line string is rejected and left at 0 rather than being partially parsed to 123. Signed-off-by: Yu Yi <yiyu@google.com> (cherry picked from commit db8c436)
baizhenyu
force-pushed
the
backport/stackdriver-overread-to-4.2
branch
from
July 28, 2026 21:32
573529f to
be2a8bc
Compare
JeffLuoo
approved these changes
Jul 29, 2026
3 tasks
zanarellidev
added a commit
to zanarellidev/fluent-bit
that referenced
this pull request
Jul 31, 2026
…trings 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 (fluent#12022, backported in fluent#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 fluent#12073, but it was self-closed without a fix landing; the vulnerable code is still present at HEAD. Signed-off-by: zanarelli <zanarelli.dev@gmail.com>
zanarellidev
added a commit
to zanarellidev/fluent-bit
that referenced
this pull request
Jul 31, 2026
…trings 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 (fluent#12022, backported in fluent#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 fluent#12073, but it was self-closed without a fix landing; the vulnerable code is still present at HEAD. Signed-off-by: zanarelli <zanarelli.dev@gmail.com>
zanarellidev
added a commit
to zanarellidev/fluent-bit
that referenced
this pull request
Jul 31, 2026
…trings 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 (fluent#12022, backported in fluent#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 fluent#12073, but it was self-closed without a fix landing; the vulnerable code is still present at HEAD. Signed-off-by: zanarelli <zanarelli.dev@gmail.com>
4 tasks
edsiper
pushed a commit
that referenced
this pull request
Aug 5, 2026
…trings 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 <zanarelli.dev@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backporting #12022
Backports both commits from #12022 to 4.2:
88afc49dout_stackdriver: fix payload parsing over-reads and null safetydb8c436cout_stackdriver: reject malformed numeric subfield stringstry_assign_subfield_int()calledatoll(obj.via.str.ptr)directly on a msgpack string object. msgpack strings are not NUL-terminated -- they are a pointer into the raw chunk buffer plus an explicitsize-- soatoll()read past the end of the value until it happened to hit a non-digit byte. WhensourceLocation.line(or a numerichttpRequestsubfield) is supplied as a string, this is an out-of-bounds read that can silently corrupt the emitted integer with adjacent heap bytes.Both cherry-picks applied cleanly. After this change all five
plugins/out_stackdriver/files are byte-identical to master.Affected fields, all reached via
try_assign_subfield_int():logging.googleapis.com/sourceLocationlinehttpRequestcacheFillByteshttpRequestrequestSizehttpRequestresponseSizehttpRequeststatus4.2 is currently the only maintained branch below 5.0 that has had releases cut after these fixes landed on master (v4.2.7 on Jul 8, 4.2.8 on Jul 9) without picking them up. The fixes are presently only in the 5.0 line, first released in v5.0.9.
Enter
[N/A]in the box, if an item is not applicable to your change.Testing
Before we can approve your change; please submit the following in a comment:
Covered by the runtime test
sourceLocation_line_invalid_stringadded indb8c436c, which asserts a"123abc"line string is rejected and left at 0. ExistingsourceLocationandhttpRequestruntime tests cover the valid-string paths.If this is a change to packaging of containers or native binaries then please confirm it works for all targets.
ok-package-testlabel to test for all targets (requires maintainer to do).Documentation
Backporting
Fluent Bit is licensed under Apache 2.0, by submitting this pull request I understand that this code will be released under the terms of that license.