iemejia commented on code in PR #3853:
URL: https://github.com/apache/avro/pull/3853#discussion_r3564713468
##########
lang/c/src/codec.c:
##########
@@ -46,6 +47,31 @@
#define DEFAULT_BLOCK_SIZE (16 * 1024)
+/*
+ * When reading a data file, each 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. To guard against
+ * unbounded allocation, the decompressed size of a single block is capped.
This
+ * mirrors the Java SDK's decompression limit (AVRO-4247). The default can be
+ * overridden with the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ */
+#define AVRO_DEFAULT_MAX_DECOMPRESS_LENGTH ((int64_t) 200 * 1024 * 1024)
/* 200 MiB */
+
+static int64_t
+avro_max_decompress_length(void)
+{
+ const char *env = getenv("AVRO_MAX_DECOMPRESS_LENGTH");
+ if (env != NULL && *env != '\0') {
+ char *end = NULL;
+ long long value = strtoll(env, &end, 10);
+ if (end != NULL && *end == '\0' && value > 0) {
+ return (int64_t) value;
+ }
+ }
+ return AVRO_DEFAULT_MAX_DECOMPRESS_LENGTH;
Review Comment:
Fixed. avro_max_decompress_length() now clamps the parsed value to SIZE_MAX
(and checks errno), so it is always safe as an allocation cap and cannot
truncate when passed to avro_malloc/avro_realloc on 32-bit. Pushed in d280eaf.
##########
lang/c/src/codec.c:
##########
@@ -304,6 +347,7 @@ 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);
Review Comment:
Fixed. decode_deflate caps its working max_len to UINT_MAX, so the avail_out
update (a uInt) can no longer truncate and leave zlib's state inconsistent.
Pushed in d280eaf.
##########
lang/c/src/codec.c:
##########
@@ -339,13 +392,30 @@ static int decode_deflate(avro_codec_t c, void * data,
int64_t len)
return 1;
}
- // The buffer was not big enough. resize it.
+ // The buffer was not big enough. resize it, without growing
beyond
+ // the configured maximum decompressed size.
if (err == Z_BUF_ERROR)
{
- c->block_data = avro_realloc(c->block_data,
c->block_size, c->block_size * 2);
- s->next_out = c->block_data + s->total_out;
- s->avail_out += c->block_size;
- c->block_size = c->block_size * 2;
+ int64_t new_size = c->block_size * 2;
+ if (new_size > max_len) {
+ new_size = max_len;
+ }
Review Comment:
Fixed. The deflate growth now doubles with a checked pattern (compare
block_size against max_len/2 before multiplying), avoiding signed overflow. The
UINT_MAX cap above also bounds block_size well below the overflow range. Pushed
in d280eaf.
##########
lang/c/src/codec.c:
##########
@@ -483,11 +554,33 @@ static int decode_lzma(avro_codec_t codec, void * data,
int64_t len)
codec->used_size = write_pos;
- // If it ran out of space to decode, give it more!!
- // It will continue where it left off because of read_pos and
write_pos.
+ // Reject a block that decompresses to more than the allowed
maximum,
+ // to guard against unbounded allocation from a high-ratio
block.
+ if ((int64_t) write_pos > max_len) {
+ avro_set_error("Decompressed block size exceeds the
maximum allowed of %lld bytes",
+ (long long) max_len);
+ return 1;
+ }
+
+ // If it ran out of space to decode, give it more (without
growing
+ // beyond the configured maximum decompressed size).
if (ret == LZMA_BUF_ERROR) {
- codec->block_data = avro_realloc(codec->block_data,
codec->block_size, codec->block_size * 2);
- codec->block_size = codec->block_size * 2;
+ int64_t new_size = codec->block_size * 2;
+ if (new_size > max_len) {
+ new_size = max_len;
+ }
Review Comment:
Fixed. decode_lzma uses the same checked doubling (compare block_size
against max_len/2 before multiplying) to avoid signed overflow. Pushed in
d280eaf.
--
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]
