Skip to content

out_stackdriver: fix multiple memory leaks and potential corruption #11667

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
zkdlin211 wants to merge 4 commits into
base: master
Choose a base branch
from
Open

out_stackdriver: fix multiple memory leaks and potential corruption #11667

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2f85893
out_stackdriver: fix multiple memory leaks and potential corruption.
zkdlin211 Apr 3, 2026
d4c6c35
Merge branch 'fluent:master' into master
zkdlin211 Apr 3, 2026
6440aed
Add write permissions for packages in integration test
zkdlin211 Apr 3, 2026
1c23f97
Update permissions in master integration test workflow
zkdlin211 Apr 3, 2026
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
1 change: 1 addition & 0 deletions .github/workflows/master-integration-test.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jobs:
needs: master-integration-test-build
permissions:
contents: read
packages: write
uses: ./.github/workflows/call-run-integration-test.yaml
with:
image_name: ghcr.io/${{ github.repository }}/master
Expand Down
18 changes: 9 additions & 9 deletions plugins/out_stackdriver/gce_metadata.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@

static int fetch_metadata(struct flb_stackdriver *ctx,
struct flb_upstream *upstream, char *uri,
char *payload)
flb_sds_t *payload)
{
int ret;
int ret_code;
Expand All @@ -44,15 +44,15 @@ static int fetch_metadata(struct flb_stackdriver *ctx,
/* If runtime test mode is enabled, add test data */
if (ctx->ins->test_mode == FLB_TRUE) {
if (strcmp(uri, FLB_STD_METADATA_PROJECT_ID_URI) == 0) {
flb_sds_cat(payload, "fluent-bit-test", 15);
*payload = flb_sds_cat(*payload, "fluent-bit-test", 15);
return 0;
}
else if (strcmp(uri, FLB_STD_METADATA_ZONE_URI) == 0) {
flb_sds_cat(payload, "projects/0123456789/zones/fluent", 32);
*payload = flb_sds_cat(*payload, "projects/0123456789/zones/fluent", 32);
return 0;
}
else if (strcmp(uri, FLB_STD_METADATA_INSTANCE_ID_URI) == 0) {
flb_sds_cat(payload, "333222111", 9);
*payload = flb_sds_cat(*payload, "333222111", 9);
Comment on lines 46 to +55

@coderabbitai coderabbitai Bot Apr 3, 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.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sds_file="$(fd -i '^flb_sds\.c$' | head -n1)"
if [ -z "${sds_file}" ]; then
  echo "flb_sds.c not found" >&2
  exit 1
fi

echo "== flb_sds_cat / flb_sds_copy definitions =="
rg -n -C4 'flb_sds_(cat|copy)\s*\(' "$sds_file"

echo
echo "== NULL-return paths in flb_sds.c =="
rg -n -C2 'return NULL;' "$sds_file"

echo
echo "== Changed assignments in fetch_metadata =="
rg -n -C2 'flb_sds_(cat|copy)\s*\(\*payload' plugins/out_stackdriver/gce_metadata.c

echo
echo "== Downstream payload dereferences after fetch_metadata success =="
rg -n -C2 'flb_sds_len\(payload\)|flb_sds_create\(payload\)' plugins/out_stackdriver/gce_metadata.c

Repository: fluent/fluent-bit

Length of output: 4311

Handle SDS growth failures before returning success.

Lines 47, 51, 55, and 91 assign the result of flb_sds_cat() or flb_sds_copy() directly to *payload without checking for allocation failures. Both functions return NULL on reallocation failure. If this occurs, *payload becomes NULL, but the function returns 0 (success). Downstream code then dereferences the NULL pointer via flb_sds_len(payload), flb_sds_create(payload), or loop conditions, causing a crash.

The codebase already has flb_sds_cat_safe() implementing the correct pattern: validate the result before reassigning to the caller's buffer and return an error code on failure.

💡 Suggested fix pattern (from flb_sds_cat_safe) 
-        if (c->resp.status == 200) {
-            ret_code = 0;
-            *payload = flb_sds_copy(*payload, c->resp.payload, c->resp.payload_size);
-        }
+        if (c->resp.status == 200) {
+            flb_sds_t tmp;
+            tmp = flb_sds_copy(*payload, c->resp.payload, c->resp.payload_size);
+            if (!tmp) {
+                flb_errno();
+                ret_code = -1;
+            }
+            else {
+                *payload = tmp;
+                ret_code = 0;
+            }
+        }

Apply the same validation to all three flb_sds_cat() calls in test mode (lines 47, 51, 55).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@plugins/out_stackdriver/gce_metadata.c` around lines 46 - 55, The test-mode
branches that append/copy into *payload (calls to flb_sds_cat() for
FLB_STD_METADATA_PROJECT_ID_URI, FLB_STD_METADATA_ZONE_URI,
FLB_STD_METADATA_INSTANCE_ID_URI and the flb_sds_copy() use around line 91) must
validate the return value before overwriting *payload: store the
flb_sds_cat()/flb_sds_copy() result in a temporary variable, check for NULL, and
if NULL return a non-zero error (e.g. -1) instead of returning success;
alternatively call the existing flb_sds_cat_safe()/flb_sds_copy_safe() pattern.
Update the branches handling FLB_STD_METADATA_PROJECT_ID_URI,
FLB_STD_METADATA_ZONE_URI, FLB_STD_METADATA_INSTANCE_ID_URI (and the copy at
line ~91) to follow this pattern so allocation failures do not leave *payload
NULL.

return 0;
}
return -1;
Expand Down Expand Up @@ -88,7 +88,7 @@ static int fetch_metadata(struct flb_stackdriver *ctx,
flb_plg_debug(ctx->ins, "HTTP Status=%i", c->resp.status);
if (c->resp.status == 200) {
ret_code = 0;
flb_sds_copy(payload, c->resp.payload, c->resp.payload_size);
*payload = flb_sds_copy(*payload, c->resp.payload, c->resp.payload_size);

@chatgpt-codex-connector chatgpt-codex-connector Bot Apr 3, 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.

P1 Badge Propagate SDS copy allocation failures

After switching fetch_metadata to update the caller's SDS pointer, this assignment can set *payload to NULL when flb_sds_copy fails (e.g., realloc under memory pressure), but the function still leaves ret_code as success. Callers then continue on the success path and dereference payload (flb_sds_len, parsing, destroy), which can crash and also loses the original SDS pointer. Treat a NULL return from flb_sds_copy as an error and return failure from fetch_metadata.

Useful? React with 👍 / 👎.

}
else {
if (c->resp.payload_size > 0) {
Expand Down Expand Up @@ -117,7 +117,7 @@ int gce_metadata_read_token(struct flb_stackdriver *ctx)

uri = flb_sds_cat(uri, ctx->client_email, flb_sds_len(ctx->client_email));
uri = flb_sds_cat(uri, "/token", 6);
ret = fetch_metadata(ctx, ctx->metadata_u, uri, payload);
ret = fetch_metadata(ctx, ctx->metadata_u, uri, &payload);
if (ret != 0) {
flb_plg_error(ctx->ins, "can't fetch token from the metadata server");
flb_sds_destroy(payload);
Expand Down Expand Up @@ -147,7 +147,7 @@ int gce_metadata_read_zone(struct flb_stackdriver *ctx)
flb_sds_t zone = NULL;

ret = fetch_metadata(ctx, ctx->metadata_u, FLB_STD_METADATA_ZONE_URI,
payload);
&payload);
if (ret != 0) {
flb_plg_error(ctx->ins, "can't fetch zone from the metadata server");
flb_sds_destroy(payload);
Expand Down Expand Up @@ -193,7 +193,7 @@ int gce_metadata_read_project_id(struct flb_stackdriver *ctx)
flb_sds_t payload = flb_sds_create_size(4096);

ret = fetch_metadata(ctx, ctx->metadata_u,
FLB_STD_METADATA_PROJECT_ID_URI, payload);
FLB_STD_METADATA_PROJECT_ID_URI, &payload);
if (ret != 0) {
flb_plg_error(ctx->ins, "can't fetch project id from the metadata server");
flb_sds_destroy(payload);
Expand All @@ -210,7 +210,7 @@ int gce_metadata_read_instance_id(struct flb_stackdriver *ctx)
flb_sds_t payload = flb_sds_create_size(4096);

ret = fetch_metadata(ctx, ctx->metadata_u,
FLB_STD_METADATA_INSTANCE_ID_URI, payload);
FLB_STD_METADATA_INSTANCE_ID_URI, &payload);
if (ret != 0) {
flb_plg_error(ctx->ins, "can't fetch instance id from the metadata server");
flb_sds_destroy(payload);
Expand Down
5 changes: 5 additions & 0 deletions plugins/out_stackdriver/stackdriver.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,9 @@ static int pack_resource_labels(struct flb_stackdriver *ctx,
} else {
flb_plg_warn(ctx->ins, "failed to find a corresponding entry for "
"resource label entry [%s=%s]", label_kv->key, label_kv->val);
if (rval) {
flb_ra_key_value_destroy(rval);
}
}
flb_ra_destroy(ra);
} else {
Expand Down Expand Up @@ -2476,6 +2479,8 @@ static flb_sds_t stackdriver_format(struct flb_stackdriver *ctx,
flb_sds_destroy(log_name);
}

destroy_http_request(&http_request);

flb_log_event_decoder_destroy(&log_decoder);
msgpack_sbuffer_destroy(&mp_sbuf);

Expand Down
3 changes: 3 additions & 0 deletions plugins/out_stackdriver/stackdriver_conf.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ static int read_credentials_file(const char *cred_file, struct flb_stackdriver *
ctx->creds->type = flb_sds_create_len(val, val_len);
}
else if (key_cmp(key, key_len, "project_id") == 0) {
if (ctx->project_id) {
flb_sds_destroy(ctx->project_id);
}
ctx->project_id = flb_sds_create_len(val, val_len);
}
else if (key_cmp(key, key_len, "private_key_id") == 0) {
Expand Down
Loading