From 34abaadb9a705d6aa092ff8c9722720fbedd064d Mon Sep 17 00:00:00 2001 From: TapTap Date: Mon, 14 Sep 2026 17:11:02 +0200 Subject: [PATCH] fix: address low/informational sec-parser follow-ups - client_cli: capture errno before output_escape() in read_patterns_from_file() so an over-long line is still reported as EFBIG instead of the (possibly malloc-clobbered) errno. - file_list: guard string_list_add() capacity doubling against overflow (capacity > INT_MAX / 2), matching filter_rule_list_add(); callers already surface the false as a memory-allocation error. - compression: ZSTD_isError() is true for ZSTD_CONTENTSIZE_UNKNOWN, which made the 3x unknown-size fallback dead code. Test the CONTENTSIZE_ERROR/UNKNOWN sentinels explicitly so unknown-size frames reach the estimate path (still bounded by the existing hard limit) while invalid frames are rejected. Known-size frames and the 100 MB ceiling/overflow checks are unchanged. - tests: add an unknown-content-size-frame decompression test. Tests: ./build/tests and ./build-asan/tests all pass (42/42); clang-format + cppcheck clean. --- src/client/client_cli.c | 7 +++-- src/shared/compression.c | 10 +++++-- src/shared/file_list.c | 2 ++ tests/test_compression.c | 59 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/src/client/client_cli.c b/src/client/client_cli.c index 6a2abba..b8637c3 100644 --- a/src/client/client_cli.c +++ b/src/client/client_cli.c @@ -2045,13 +2045,16 @@ static int read_patterns_from_file(const char* filepath, char*** patterns, int* while (true) { ssize_t n = utils_getdelim_bounded(fp, &line, &line_size, '\n', UTILS_MAX_LINE_LEN); if (n < 0) { + /* output_escape() may allocate (and clobber errno): capture the reader's + * errno first so an over-long line is still reported as EFBIG. */ + int saved_errno = errno; char* escaped = output_escape(filepath, false); - if (errno == EFBIG) { + if (saved_errno == EFBIG) { log_message(LOG_LEVEL_ERROR, "pattern file '%s' has a line exceeding %d bytes", escaped ? escaped : "", (int)UTILS_MAX_LINE_LEN); } else { log_message(LOG_LEVEL_ERROR, "could not read pattern file '%s': %s", - escaped ? escaped : "", strerror(errno)); + escaped ? escaped : "", strerror(saved_errno)); } free(escaped); free(line); diff --git a/src/shared/compression.c b/src/shared/compression.c index c40c534..dad630c 100644 --- a/src/shared/compression.c +++ b/src/shared/compression.c @@ -243,9 +243,13 @@ Data* data_decompress_limited(Data* compressed_data, size_t maximum_size) { log_debug_message(LOG_DEBUG_UTIL, "Start to decompress data"); unsigned long long dst_size = ZSTD_getFrameContentSize(compressed_data->data, compressed_data->size); - if (ZSTD_isError(dst_size)) { - log_message(LOG_LEVEL_ERROR, "Failed to get decompressed size: %s", - ZSTD_getErrorName(dst_size)); + /* ZSTD_isError() is also true for ZSTD_CONTENTSIZE_ERROR and + * ZSTD_CONTENTSIZE_UNKNOWN (both are encoded near (size_t)-1), so test the + * sentinels explicitly instead of blanket-rejecting every error-ish value: + * only CONTENTSIZE_ERROR means an unreadable header, while CONTENTSIZE_UNKNOWN + * must reach the estimate fallback below. */ + if (dst_size == ZSTD_CONTENTSIZE_ERROR) { + log_message(LOG_LEVEL_ERROR, "Failed to get decompressed size: invalid zstd frame"); return NULL; } diff --git a/src/shared/file_list.c b/src/shared/file_list.c index fc04e0b..5b2a138 100644 --- a/src/shared/file_list.c +++ b/src/shared/file_list.c @@ -23,6 +23,8 @@ static void string_list_destroy(StringList* list) { static bool string_list_add(StringList* list, const char* text) { if (list->count == list->capacity) { + if (list->capacity > INT_MAX / 2) + return false; int new_cap = list->capacity > 0 ? list->capacity * 2 : 16; char** grown = realloc(list->items, (size_t)new_cap * sizeof(char*)); if (!grown) diff --git a/tests/test_compression.c b/tests/test_compression.c index 2ebb8da..063f060 100644 --- a/tests/test_compression.c +++ b/tests/test_compression.c @@ -9,6 +9,7 @@ #include #include #include +#include static void test_data_compress_decompress_roundtrip() { const char original[] = "Hello, World! This is test data for compression round-trip!"; @@ -139,6 +140,63 @@ static void test_chunk_compress_decompress_roundtrip() { unlink(path2); } +/* Build a zstd frame whose header omits the content size (the content size + * flag is cleared), which ZSTD_getFrameContentSize reports as + * ZSTD_CONTENTSIZE_UNKNOWN. */ +static Data* make_unknown_size_frame(const void* src, size_t len) { + ZSTD_CCtx* cctx = ZSTD_createCCtx(); + if (!cctx) + return NULL; + ZSTD_CCtx_setParameter(cctx, ZSTD_c_contentSizeFlag, 0); + size_t cap = ZSTD_compressBound(len); + Data* out = data_create_empty(cap); + if (!out) { + ZSTD_freeCCtx(cctx); + return NULL; + } + ZSTD_inBuffer in = {src, len, 0}; + ZSTD_outBuffer ob = {out->data, cap, 0}; + size_t ret; + do { + ret = ZSTD_compressStream2(cctx, &ob, &in, ZSTD_e_end); + if (ZSTD_isError(ret)) { + data_destroy(out); + ZSTD_freeCCtx(cctx); + return NULL; + } + } while (ret > 0); + out->size = ob.pos; + ZSTD_freeCCtx(cctx); + return out; +} + +/* ZSTD_CONTENTSIZE_UNKNOWN is flagged by ZSTD_isError(), so a naive + * ZSTD_isError() check rejects every unknown-size frame. Such a frame must + * instead reach the 3x estimate fallback and decompress correctly. */ +static void test_data_decompress_unknown_size_frame() { + const char original[] = "unknown-content-size frame: the decompressor must use the 3x estimate, " + "not reject the frame as an error."; + size_t len = strlen(original); + char* buf = malloc(len); + EXPECT_NOT_NULL(buf); + memcpy(buf, original, len); + + Data* frame = make_unknown_size_frame(buf, len); + free(buf); + EXPECT_NOT_NULL(frame); + /* Guard the premise of the test: the frame really has no stored size. */ + EXPECT_EQ_INT((int)ZSTD_getFrameContentSize(frame->data, frame->size), + (int)ZSTD_CONTENTSIZE_UNKNOWN); + + Data* decompressed = data_decompress(frame); + EXPECT_NOT_NULL(decompressed); + EXPECT_EQ_INT((int)decompressed->size, (int)len); + EXPECT_EQ_INT(memcmp(decompressed->data, original, len), 0); + + data_destroy(decompressed); + data_destroy(frame); +} + typedef struct { int id; int iterations; @@ -252,6 +310,7 @@ static void test_data_decompress_truncated_frame_fails() { void test_compression() { test_data_compress_decompress_roundtrip(); test_data_compress_decompress_large(); + test_data_decompress_unknown_size_frame(); test_data_decompress_truncated_frame_fails(); test_skip_compress_suffix_matching(); test_data_compress_with_threads_roundtrip();