fluent / fluent/fluent-bit

Direct usage of `flb_sds_alloc` as size argument for `flb_sds_snprintf`

Open
#9,803 5 comments 0 reactions 0 assignees View on GitHub
exempt-stale status: waiting-for-triage
Dominant language
C
Stars
8.1k
Forks
2k
Avg merge
4d 20h
Merged PRs (30d)
71

Description

## 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](https://cplusplus.com/reference/cstdio/vsnprintf/). 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:
```c
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: https://github.com/fluent/fluent-bit/pull/9779#discussion_r1904382972

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:
https://github.com/fluent/fluent-bit/blob/68f1887f46547561213db2e1e89571b04d3b8594/plugins/out_es/es.c#L366
https://github.com/fluent/fluent-bit/blob/68f1887f46547561213db2e1e89571b04d3b8594/plugins/out_azure_logs_ingestion/azure_logs_ingestion_conf.c#L102
https://github.com/fluent/fluent-bit/blob/68f1887f46547561213db2e1e89571b04d3b8594/plugins/out_azure_kusto/azure_kusto.c#L152

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

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.