Skip to content

Patch librdkafka cluster_id strlen() overrun in DescribeCluster parsing - #24437

Draft
piochelepiotr wants to merge 3 commits into
masterfrom
piotr.wolski/librdkafka-fix-cluster-id-strlen
Draft

Patch librdkafka cluster_id strlen() overrun in DescribeCluster parsing#24437
piochelepiotr wants to merge 3 commits into
masterfrom
piotr.wolski/librdkafka-fix-cluster-id-strlen

Conversation

@piochelepiotr

Copy link
Copy Markdown
Contributor

What does this PR do?

Applies a build-time patch to librdkafka fixing a buffer overrun in the admin DescribeCluster metadata-response parsing path (rd_kafka_parse_Metadata() in rdkafka_metadata.c).

The cluster_id field is read as a length-prefixed Kafka wire string (rd_kafkap_str_t) that points directly into the network receive buffer and is not nul-terminated — the wire format has no terminator, the next field (controller_id) simply follows immediately. The existing code copies it with rd_tmpabuf_write_str(), which determines the copy length via strlen(). Since cluster_id.str isn't nul-terminated at its real end, strlen() reads past it into the adjacent controller_id bytes (and beyond), stopping only when it happens to hit an accidental zero byte.

For a typical Kafka cluster with small numeric broker/controller IDs (e.g. 0, 1, 2), the big-endian encoding starts with 00 00 00 0N — an accidental null byte right after the string that "accidentally" terminates it correctly, masking the bug. Against a WarpStream-backed cluster, whose controller/broker ID is a large, non-sequential value (observed: 1611705428, hex 60 10 AC 54 — no zero bytes), strlen() runs past the intended string into unrelated binary data. The corrupted, non-UTF-8-safe result is then passed to PyUnicode_FromString() inside confluent-kafka-python's cfl_PyDict_SetString(), which fails and returns NULL — a NULL that cfl_PyDict_SetString() never checks before handing it to PyDict_SetItem(), causing a segfault (exit code 139) inside the Datadog Agent's kafka_consumer integration whenever it calls AdminClient.describe_cluster() against such a broker.

The fix copies cluster_id using its wire-known length (via rd_tmpabuf_write()) instead of relying on strlen(), mirroring the existing, already-correct rd_kafka_buf_read_str_tmpabuf() pattern used elsewhere in the same function for broker host/rack strings.

This mirrors the existing precedent in #23240: applying an upstream-style fix as a build-time patch via the install-from-source.sh PATCHES mechanism, since this fix is not yet available in a released librdkafka version.

Changes

  • .builders/patches/librdkafka-fix-cluster-id-strlen.patch — the fix, verified to apply cleanly against librdkafka v2.13.2 (the version pinned via confluent-kafka==2.13.2 in kafka_consumer)
  • .builders/images/linux-x86_64/build_script.sh — apply patch during librdkafka build
  • .builders/images/linux-aarch64/build_script.sh — same

Motivation

Root-caused a reported segfault (exit code 139) when the kafka_consumer integration calls AdminClient.describe_cluster() against a WarpStream-backed Kafka cluster. Confirmed via:

  • Deterministic reproduction inside a Linux/glibc container matching production
  • valgrind + ARM64 register-level gdb inspection proving a genuine NULL-pointer dereference in PyDict_SetItem
  • Independent raw Kafka-wire-protocol probing proving the cluster_id bytes on the wire are always valid and correctly encoded (ruling out server-side/WarpStream corruption)
  • Live gdb breakpoint at cfl_PyDict_SetString()'s entry, dumping the actual val argument and observing the valid cluster_id string immediately followed — with no null terminator — by the raw bytes of controller_id
  • Source-level confirmation in librdkafka that rd_tmpabuf_write_str() uses strlen() on a non-nul-terminated wire string, while the sibling helper rd_kafka_buf_read_str_tmpabuf() (used for other fields in the same function) correctly uses the wire-known length

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add qa/required if this PR needs QA validation, or qa/skip-qa if it does not. Exactly one of the two is required.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

…uster metadata parsing

rd_kafka_parse_Metadata() copies the admin DescribeCluster cluster_id
via rd_tmpabuf_write_str(), which determines length with strlen() on a
wire string that is not nul-terminated. The bytes immediately following
it on the wire are the next field (controller_id), so the copy silently
overruns into that field whenever its big-endian encoding doesn't
happen to contain an early nul byte (e.g. a large controller/broker
id, as WarpStream assigns). The corrupted, non-UTF-8 result then makes
PyUnicode_FromString() fail inside confluent-kafka-python's unchecked
cfl_PyDict_SetString(), crashing the process with a NULL-pointer
segfault in PyDict_SetItem().

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@piochelepiotr piochelepiotr added the qa/required QA is required for this PR and will generate a QA card label Jul 8, 2026
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@piochelepiotr

Copy link
Copy Markdown
Contributor Author

Found the identical bug filed upstream today: confluentinc/librdkafka#5545 ("Fix cluster id over-read past the controller id in Metadata response"). Its root-cause writeup matches this investigation exactly (down to identifying controller_id's big-endian encoding as the trigger byte). I've updated .builders/patches/librdkafka-fix-cluster-id-strlen.patch to match that PR's fix verbatim (rd_tmpabuf_alloc() + memcpy() with an explicit nul terminator), since it's author-reviewed and ships with a regression test (do_test_cluster_id_not_overread in tests/0146-metadata_mock.c).

Verified locally: built librdkafka v2.13.2 from source with this exact patch applied, rebuilt confluent-kafka==2.13.2 against it, and re-ran the crash repro against the real WarpStream cluster.

  • Before: describe_cluster() segfaults (exit 139), every time.
  • After: describe_cluster() succeeds, returning the correct cluster_id (vci_f9150c0e_3cbc_4c6c_a519_23e7fec74edb, matching list_topics()), correct controller (1611705428), and correct nodes.

@dd-octo-sts

dd-octo-sts Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Validation Report

All 21 validations passed.

Show details
Validation Description Status
agent-reqs Verify check versions match the Agent requirements file
ci Validate CI configuration and code coverage settings
codeowners Validate every integration has a CODEOWNERS entry
config Validate default configuration files against spec.yaml
dep Verify dependency pins are consistent and Agent-compatible
http Validate integrations use the HTTP wrapper correctly
imports Validate check imports do not use deprecated modules
integration-style Validate check code style conventions
jmx-metrics Validate JMX metrics definition files and config
labeler Validate PR labeler config matches integration directories
legacy-signature Validate no integration uses the legacy Agent check signature
license-headers Validate Python files have proper license headers
licenses Validate third-party license attribution list
metadata Validate metadata.csv metric definitions
models Validate configuration data models match spec.yaml
openmetrics Validate OpenMetrics integrations disable the metric limit
package Validate Python package metadata and naming
qa-label Validate the pull request declares whether it needs QA for the next Agent release
readmes Validate README files have required sections
saved-views Validate saved view JSON file structure and fields
version Validate version consistency between package and changelog

View full run

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant