Skip to content
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.

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

Direct usage of flb_sds_alloc as size argument for flb_sds_snprintf #9803

Open
braydonk opened this issue Jan 7, 2025 · 0 comments
Open

Comments

@braydonk
Copy link
Contributor

braydonk commented Jan 7, 2025

Bug Report

Describe the bug
There are numerous spots throughout the codebase where flb_sds_alloc is used as the size argument for flb_sds_snprintf. flb_sds_alloc gets the alloc field of struct flb_sds, which does not include the null terminator, however vsnprintf expects a size argument that does include the null terminator. This leads to a potential bug where the result of flb_sds_snprintf could include one less character than expected.

To Reproduce
In a random test file that included flb_sds.h, I wrote the following test:

static void test_snprintf()
{
    flb_sds_t s = flb_sds_create_size(4);
    flb_sds_snprintf(&s, flb_sds_alloc(s)+1, "%s", "test");
    printf("\n%s\n", s);
    TEST_ASSERT(strcmp(s, "test") == 0);
}

The result was this:

$ /usr/local/google/home/braydonk/Git/fluent-bit/build/bin/flb-it-csv snprintf
Test snprintf...                                
test
[ OK ]
SUCCESS: All unit tests have passed.

When I change it to not adjusting the size (i.e. instead of flb_sds_alloc(s)+1 it's just flb_sds_alloc(s)):

braydonk@bk:~/Git/fluent-bit/build/bin$ /usr/local/google/home/braydonk/Git/fluent-bit/build/bin/flb-it-csv snprintf
Test snprintf...                                
tes
[ FAILED ]
  csv.c:169: Check strcmp(s, "test") == 0... failed
FAILED: 1 of 1 unit tests has failed.

Additional context
First discovered by @jefferbrecht here: #9779 (comment)

Searching in this repo for flb_sds_snprintf usage has some examples right on the first page, but here is a quick list of a few spots doing it as an example:

index_len = flb_sds_snprintf(&j_index,

flb_sds_snprintf(&ctx->auth_url, flb_sds_alloc(ctx->auth_url),

flb_sds_snprintf(&body, flb_sds_alloc(body),

This is unlikely to be an issue of safety/exposure to a vulnerability thanks to snprintf safely discarding any characters past <size argument> - 1.

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

No branches or pull requests

1 participant