Skip to content

snappy: fix intermittent decompression failures for raw snappy data #11550

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

snappy: fix intermittent decompression failures for raw snappy data #11550

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7800ae2
snappy: fix raw block data misidentified as framed
thewillyhuman Mar 13, 2026
d1b8f03
snappy: fix multi-chunk framed data silently dropped
thewillyhuman Mar 13, 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
31 changes: 28 additions & 3 deletions src/flb_snappy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

#include <snappy.h>

#include <string.h>

int flb_snappy_compress(char *in_data, size_t in_len,
char **out_data, size_t *out_len)
{
Expand Down Expand Up @@ -139,7 +141,25 @@ int flb_snappy_uncompress_framed_data(char *in_data, size_t in_len,
struct cfl_list chunks;
struct flb_snappy_data_chunk *chunk;

if (*((uint8_t *) in_data) != FLB_SNAPPY_FRAME_TYPE_STREAM_IDENTIFIER) {
/*
* Distinguish raw/block snappy from the framed/streaming format.
*
* The framed format always starts with a stream identifier frame:
* byte 0: 0xFF (stream identifier chunk type)
* bytes 1-3: 0x06 0x00 0x00 (length = 6, little-endian 24-bit)
* bytes 4-9: "sNaPpY"
*
* A single first-byte check is insufficient because raw snappy data
* encodes the uncompressed length as a varint whose first byte can
* be 0xFF (e.g. when the uncompressed size mod 128 == 127 and >= 128).
* Checking the full 10-byte header avoids this false positive.
*/
if (in_len < 10 ||
*((uint8_t *) &in_data[0]) != 0xFF ||
*((uint8_t *) &in_data[1]) != 0x06 ||
*((uint8_t *) &in_data[2]) != 0x00 ||
*((uint8_t *) &in_data[3]) != 0x00 ||
memcmp(&in_data[4], FLB_SNAPPY_STREAM_IDENTIFIER_STRING, 6) != 0) {
return flb_snappy_uncompress(in_data, in_len, out_data, out_len);
}

Expand Down Expand Up @@ -282,7 +302,6 @@ int flb_snappy_uncompress_framed_data(char *in_data, size_t in_len,
}

aggregated_data_buffer = NULL;
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Prevent leaking output buffer when framed parse fails

Removing the aggregated_data_length = 0 reset fixes multi-chunk success cases, but it also means aggregated_data_length can stay non-zero when a later frame sets result != 0 (for example, checksum mismatch after one valid chunk). In that case the non-fast path still allocates aggregated_data_buffer and returns an error, and current callers treat nonzero returns as failure without freeing *out_data, so malformed multi-frame snappy payloads can now leak memory per request. Please gate allocation on result == 0 (or clear the length on error) before setting *out_data.

Useful? React with 👍 / 👎.

Copy link
Author

@thewillyhuman thewillyhuman Mar 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated commit to take into account this case. This is completely fair point, we discussed internally but this review confirmed it.

aggregated_data_length = 0;

if (compressed_chunk_count == 1 &&
uncompressed_chunk_count == 0 &&
Expand All @@ -302,7 +321,7 @@ int flb_snappy_uncompress_framed_data(char *in_data, size_t in_len,
flb_free(chunk);
}
else {
if (aggregated_data_length > 0) {
if (aggregated_data_length > 0 && result == 0) {
aggregated_data_buffer = flb_calloc(aggregated_data_length,
sizeof(char));

Expand Down Expand Up @@ -337,6 +356,12 @@ int flb_snappy_uncompress_framed_data(char *in_data, size_t in_len,
}
}

if (result != 0) {
flb_free(aggregated_data_buffer);
aggregated_data_buffer = NULL;
aggregated_data_offset = 0;
}

*out_data = (char *) aggregated_data_buffer;
*out_len = aggregated_data_offset;

Expand Down