Direct usage of `flb_sds_alloc` as size argument for `flb_sds_snprintf`
- 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
Assessment
This issue has not been assessed yet.