Copilot commented on code in PR #3853: URL: https://github.com/apache/avro/pull/3853#discussion_r3567333089
########## lang/c/tests/test_avro_4284.c: ########## @@ -0,0 +1,137 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to you under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or + * implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +/* + * AVRO-4284: a data-file block is decompressed according to the file's codec. + * A block with a very high compression ratio (or a malformed block) can expand + * to far more memory than its compressed size. Decompression must reject a + * block whose decompressed size would exceed the configured maximum, which + * these tests set to a small value via AVRO_MAX_DECOMPRESS_LENGTH. + */ + +#include <avro.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include "codec.h" + +/* Size of the highly compressible payload used to build an over-large block. */ +#define PAYLOAD_SIZE (4 * 1024 * 1024) /* 4 MiB of zeros */ +/* Decompression limit for the test, smaller than the payload. */ +#define TEST_LIMIT "1048576" /* 1 MiB */ + +/* + * Compress `payload` with the named codec and attempt to decompress it with a + * decompression limit smaller than the payload. Returns 0 if the codec is not + * available (test skipped for that codec), 1 on test failure, 2 on success. + */ +static int +check_codec_rejects_oversized(const char *name, const char *payload, int64_t payload_len) +{ + struct avro_codec_t_ codec; + memset(&codec, 0, sizeof(codec)); + + if (avro_codec(&codec, name) != 0) { + fprintf(stderr, " codec %s not available, skipping\n", name); + return 0; + } + + if (avro_codec_encode(&codec, (void *) payload, payload_len) != 0) { + fprintf(stderr, " codec %s: encode failed: %s\n", name, avro_strerror()); + avro_codec_reset(&codec); + return 1; + } + + /* Copy the compressed bytes; decode reuses the codec's block buffer. */ + int64_t compressed_len = codec.used_size; + if (compressed_len <= 0) { + fprintf(stderr, " codec %s: unexpected compressed length %lld\n", + name, (long long) compressed_len); + avro_codec_reset(&codec); + return 1; + } + char *compressed = (char *) malloc((size_t) compressed_len); + if (compressed == NULL) { + avro_codec_reset(&codec); + return 1; + } + memcpy(compressed, codec.block_data, (size_t) compressed_len); + + int rc = avro_codec_decode(&codec, compressed, compressed_len); + /* Capture the error before reset/free may clobber it. */ + char err_copy[512]; + err_copy[0] = '\0'; + if (rc != 0) { + strncpy(err_copy, avro_strerror(), sizeof(err_copy) - 1); + err_copy[sizeof(err_copy) - 1] = '\0'; + } + + free(compressed); + avro_codec_reset(&codec); + + if (rc == 0) { + fprintf(stderr, " codec %s: expected decompression to be rejected but it succeeded\n", name); + return 1; + } + /* Ensure it was rejected specifically for exceeding the size limit, not + * some unrelated failure (e.g. a CRC error or regression). */ + if (strstr(err_copy, "exceeds the maximum") == NULL) { + fprintf(stderr, " codec %s: rejected but not for the size limit: %s\n", name, err_copy); + return 1; + } + fprintf(stderr, " codec %s: over-limit block rejected as expected\n", name); + return 2; +} + +int main(void) +{ +#ifdef _WIN32 + _putenv_s("AVRO_MAX_DECOMPRESS_LENGTH", TEST_LIMIT); +#else + setenv("AVRO_MAX_DECOMPRESS_LENGTH", TEST_LIMIT, 1); +#endif Review Comment: The test sets AVRO_MAX_DECOMPRESS_LENGTH but doesn't check whether setting the environment variable succeeded. If setenv/_putenv_s fails (e.g., due to ENOMEM), the test may run with the default limit and produce misleading results. Check the return value and fail the test early if the environment can't be set. ########## lang/c/src/codec.c: ########## @@ -462,6 +556,7 @@ static int decode_lzma(avro_codec_t codec, void * data, int64_t len) { size_t read_pos = 0; size_t write_pos = 0; + int64_t max_len = avro_max_decompress_length(); lzma_ret ret; lzma_filter* filters = codec_data_lzma_filters(codec->codec_data); Review Comment: max_len is computed for lzma decode, but the initial output buffer allocation still always uses DEFAULT_BLOCK_SIZE. If AVRO_MAX_DECOMPRESS_LENGTH is set smaller than DEFAULT_BLOCK_SIZE, this allocates more than the configured cap. Allocate the initial buffer as min(DEFAULT_BLOCK_SIZE, max_len) so the limit is respected even for very small configurations. ########## lang/c/src/codec.c: ########## @@ -304,8 +359,16 @@ static int encode_deflate(avro_codec_t c, void * data, int64_t len) static int decode_deflate(avro_codec_t c, void * data, int64_t len) { int err; + int64_t max_len = avro_max_decompress_length(); z_stream *s = codec_data_inflate_stream(c->codec_data); + /* zlib's avail_out is a uInt; keep the working cap within its range so + * the buffer-growth arithmetic below cannot truncate when updating + * avail_out, which would leave zlib's state inconsistent. */ + if (max_len > (int64_t) UINT_MAX) { + max_len = (int64_t) UINT_MAX; + } + if (!c->block_data) { c->block_data = avro_malloc(DEFAULT_BLOCK_SIZE); c->block_size = DEFAULT_BLOCK_SIZE; Review Comment: The initial output buffer allocation ignores the configured max decompressed length. If AVRO_MAX_DECOMPRESS_LENGTH is set smaller than DEFAULT_BLOCK_SIZE, this still allocates DEFAULT_BLOCK_SIZE bytes, exceeding the intended cap. Allocate the initial buffer as min(DEFAULT_BLOCK_SIZE, max_len) so the limit is respected even for very small configurations. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
