From cfb76987341edfc3654391f27d768dc4176def0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 22:27:20 +0200 Subject: [PATCH 01/16] AVRO-4293: [c] Validate available bytes before allocating for length-prefixed values and collections A bytes or string value is a length prefix followed by that many bytes, and an array or map block is an element count followed by that many items. A malicious or truncated input can declare a huge length or count with little or no data, causing a correspondingly huge allocation or an unbounded append loop. - avro_reader_bytes_available() returns the bytes still readable from a memory-backed reader (or -1 when unknown). read_bytes/read_string consult it to reject an over-large declared length before allocating. - read_array_value/read_map_value reject a block whose element count could not be backed by the bytes remaining, using min_bytes_per_element() computed from the element schema so a zero-byte element type (e.g. null) is not falsely rejected. The comparison divides to avoid overflow. Mirrors the Java SDK's checks (AVRO-4241). Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/encoding.h | 10 ++ lang/c/src/encoding_binary.c | 24 ++++ lang/c/src/io.c | 14 +++ lang/c/src/value-read.c | 83 +++++++++++++ lang/c/tests/CMakeLists.txt | 1 + lang/c/tests/test_avro_4293.c | 222 ++++++++++++++++++++++++++++++++++ 6 files changed, 354 insertions(+) create mode 100644 lang/c/tests/test_avro_4293.c diff --git a/lang/c/src/encoding.h b/lang/c/src/encoding.h index b90c91dd5c8..8646f0cb9b8 100644 --- a/lang/c/src/encoding.h +++ b/lang/c/src/encoding.h @@ -104,5 +104,15 @@ typedef struct avro_encoding_t avro_encoding_t; extern const avro_encoding_t avro_binary_encoding; /* in * encoding_binary */ + +/* + * Returns the number of bytes still available to read from a memory-backed + * reader, or -1 when the amount remaining is unknown (e.g. file readers). + * Defined in io.c. Used to reject a length prefix that exceeds the data + * actually available before allocating for it, to guard against an + * out-of-memory attack from a malicious or truncated input. + */ +int64_t avro_reader_bytes_available(avro_reader_t reader); + CLOSE_EXTERN #endif diff --git a/lang/c/src/encoding_binary.c b/lang/c/src/encoding_binary.c index 459d5fb3db3..f66200c9e13 100644 --- a/lang/c/src/encoding_binary.c +++ b/lang/c/src/encoding_binary.c @@ -125,12 +125,24 @@ static int64_t size_int(avro_writer_t writer, const int32_t i) static int read_bytes(avro_reader_t reader, char **bytes, int64_t * len) { int rval; + int64_t available; check_prefix(rval, read_long(reader, len), "Cannot read bytes length: "); if (*len < 0) { avro_set_error("Invalid bytes length: %" PRId64, *len); return EINVAL; } + /* Reject a declared length that exceeds the data actually available + * before allocating for it, to guard against an out-of-memory attack + * from a malicious or truncated input. Only enforced when the reader + * can report the amount remaining. */ + available = avro_reader_bytes_available(reader); + if (available >= 0 && *len > available) { + avro_set_error("Bytes length %" PRId64 + " exceeds %" PRId64 " bytes available", + *len, available); + return EINVAL; + } *bytes = (char *) avro_malloc(*len + 1); if (!*bytes) { avro_set_error("Cannot allocate buffer for bytes value"); @@ -180,6 +192,7 @@ size_bytes(avro_writer_t writer, const char *bytes, const int64_t len) static int read_string(avro_reader_t reader, char **s, int64_t *len) { int64_t str_len = 0; + int64_t available; int rval; check_prefix(rval, read_long(reader, &str_len), "Cannot read string length: "); @@ -187,6 +200,17 @@ static int read_string(avro_reader_t reader, char **s, int64_t *len) avro_set_error("Invalid string length: %" PRId64, str_len); return EINVAL; } + /* Reject a declared length that exceeds the data actually available + * before allocating for it, to guard against an out-of-memory attack + * from a malicious or truncated input. Only enforced when the reader + * can report the amount remaining. */ + available = avro_reader_bytes_available(reader); + if (available >= 0 && str_len > available) { + avro_set_error("String length %" PRId64 + " exceeds %" PRId64 " bytes available", + str_len, available); + return EINVAL; + } *len = str_len + 1; *s = (char *) avro_malloc(*len); if (!*s) { diff --git a/lang/c/src/io.c b/lang/c/src/io.c index c1e2f5dc9b1..3d72b299311 100644 --- a/lang/c/src/io.c +++ b/lang/c/src/io.c @@ -20,6 +20,7 @@ #include "avro/errors.h" #include "avro/io.h" #include "avro_private.h" +#include "encoding.h" #include #include #include @@ -198,6 +199,19 @@ avro_read_memory(struct _avro_reader_memory_t *reader, void *buf, int64_t len) return 0; } +int64_t +avro_reader_bytes_available(avro_reader_t reader) +{ + if (is_memory_io(reader)) { + struct _avro_reader_memory_t *mem_reader = + avro_reader_to_memory(reader); + return mem_reader->len - mem_reader->read; + } + /* File readers buffer internally and cannot cheaply report how many + * bytes remain in the underlying file, so the amount is unknown. */ + return -1; +} + #define bytes_available(reader) (reader->end - reader->cur) #define buffer_reset(reader) {reader->cur = reader->end = reader->buffer;} diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index b6b6e79fadd..b44152d466e 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -23,6 +23,7 @@ #include "avro/basics.h" #include "avro/data.h" #include "avro/io.h" +#include "avro/schema.h" #include "avro/value.h" #include "avro_private.h" #include "encoding.h" @@ -38,6 +39,73 @@ static int read_value(avro_reader_t reader, avro_value_t *dest); +/* + * Minimum number of bytes a single value of the given schema can occupy on the + * wire. Used to reject an array/map block count that could not be backed by the + * bytes remaining. A type that can encode to zero bytes (null) returns 0, which + * disables the collection check for it (so an array of nulls is not falsely + * rejected). A recursion guard breaks self-referencing schemas. + */ +static int64_t +min_bytes_per_element(avro_schema_t schema, int depth) +{ + if (schema == NULL || depth > 64) { + return 0; + } + switch (avro_typeof(schema)) { + case AVRO_NULL: + return 0; + case AVRO_FLOAT: + return 4; + case AVRO_DOUBLE: + return 8; + case AVRO_FIXED: + return avro_schema_fixed_size(schema); + case AVRO_RECORD: { + size_t n = avro_schema_record_size(schema); + int64_t total = 0; + size_t i; + for (i = 0; i < n; i++) { + avro_schema_t field = + avro_schema_record_field_get_by_index(schema, i); + total += min_bytes_per_element(field, depth + 1); + } + return total; + } + case AVRO_LINK: + return min_bytes_per_element(avro_schema_link_target(schema), + depth + 1); + default: + /* boolean, int, long, bytes, string, enum, union, array, map: + * all encode to at least one byte. */ + return 1; + } +} + +/* + * Reject a collection (array or map) block whose declared element count could + * not be backed by the bytes actually remaining, before appending. Skipped when + * the per-element minimum is zero or when the reader cannot report how many + * bytes remain. The comparison avoids overflow by dividing. + */ +static int +ensure_collection_available(avro_reader_t reader, int64_t count, int64_t min_bytes) +{ + int64_t available; + if (count <= 0 || min_bytes <= 0) { + return 0; + } + available = avro_reader_bytes_available(reader); + if (available >= 0 && count > available / min_bytes) { + avro_set_error("Collection claims %" PRId64 + " elements with at least %" PRId64 + " bytes each, but only %" PRId64 " bytes available", + count, min_bytes, available); + return EINVAL; + } + return 0; +} + static int read_array_value(avro_reader_t reader, avro_value_t *dest) { @@ -46,6 +114,11 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) size_t index = 0; /* index within the entire array */ int64_t block_count; int64_t block_size; + int64_t min_bytes; + avro_schema_t array_schema = avro_value_get_schema(dest); + + min_bytes = min_bytes_per_element( + array_schema ? avro_schema_array_items(array_schema) : NULL, 0); check_prefix(rval, avro_binary_encoding. read_long(reader, &block_count), @@ -59,6 +132,8 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) "Cannot read array block size: "); } + check(rval, ensure_collection_available(reader, block_count, min_bytes)); + for (i = 0; i < (size_t) block_count; i++, index++) { avro_value_t child; @@ -83,6 +158,12 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) size_t index = 0; /* index within the entire array */ int64_t block_count; int64_t block_size; + int64_t min_bytes; + avro_schema_t map_schema = avro_value_get_schema(dest); + + /* Map keys are strings (>= 1 byte length prefix) plus the value. */ + min_bytes = 1 + min_bytes_per_element( + map_schema ? avro_schema_map_values(map_schema) : NULL, 0); check_prefix(rval, avro_binary_encoding.read_long(reader, &block_count), "Cannot read map block count: "); @@ -95,6 +176,8 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) "Cannot read map block size: "); } + check(rval, ensure_collection_available(reader, block_count, min_bytes)); + for (i = 0; i < (size_t) block_count; i++, index++) { char *key; int64_t key_size; diff --git a/lang/c/tests/CMakeLists.txt b/lang/c/tests/CMakeLists.txt index 6b4164fa740..d2d6a4d6408 100644 --- a/lang/c/tests/CMakeLists.txt +++ b/lang/c/tests/CMakeLists.txt @@ -88,3 +88,4 @@ add_avro_test_checkmem(test_avro_1691) add_avro_test_checkmem(test_avro_1906) add_avro_test_checkmem(test_avro_1904) add_avro_test_checkmem(test_avro_4246) +add_avro_test_checkmem(test_avro_4293) diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c new file mode 100644 index 00000000000..c079a6071b8 --- /dev/null +++ b/lang/c/tests/test_avro_4293.c @@ -0,0 +1,222 @@ +/* + * 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. + */ + +/* + * A bytes/string value is encoded as a length prefix followed by that many + * bytes of data. A malicious or truncated input can declare a huge length with + * little or no actual data, which previously caused a correspondingly huge + * allocation before the shortfall was noticed. When the reader knows how many + * bytes remain (a memory-backed reader), the declared length must be rejected + * before allocating for it. + */ + +#include +#include +#include +#include +#include + +/* Decodes buf against the given schema and returns the avro_value_read rc. + * A non-zero rc means the read was rejected. */ +static int try_decode(avro_value_iface_t *iface, const char *buf, size_t buf_size) +{ + int rc; + avro_value_t decoded; + avro_reader_t reader; + + rc = avro_generic_value_new(iface, &decoded); + if (rc != 0) { + fprintf(stderr, "avro_generic_value_new failed: %s\n", avro_strerror()); + return -1; + } + + reader = avro_reader_memory(buf, buf_size); + if (reader == NULL) { + fprintf(stderr, "avro_reader_memory failed\n"); + avro_value_decref(&decoded); + return -1; + } + + rc = avro_value_read(reader, &decoded); + + avro_reader_free(reader); + avro_value_decref(&decoded); + return rc; +} + +static int check_rejects_oversized(const char *schema_literal, const char *label) +{ + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + int rc; + int ret = EXIT_FAILURE; + + /* + * A length prefix declaring 127 bytes (zig-zag long 254 -> varint + * 0xFE 0x01), followed by no data at all. The reader has 0 bytes + * remaining after the prefix, so a declared length of 127 must be + * rejected before allocating. + */ + const char oversized[] = { (char) 0xFE, (char) 0x01 }; + + if (avro_schema_from_json_length(schema_literal, strlen(schema_literal), + &schema) != 0) { + fprintf(stderr, "%s: failed to parse schema: %s\n", label, avro_strerror()); + return EXIT_FAILURE; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "%s: failed to create iface\n", label); + avro_schema_decref(schema); + return EXIT_FAILURE; + } + + rc = try_decode(iface, oversized, sizeof(oversized)); + if (rc == 0) { + fprintf(stderr, "%s: FAIL - oversized length was accepted\n", label); + } else { + fprintf(stderr, "%s: oversized length rejected as expected: %s\n", + label, avro_strerror()); + ret = EXIT_SUCCESS; + } + + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return ret; +} + +static int check_accepts_valid(void) +{ + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + int rc; + int ret = EXIT_FAILURE; + const char *text = NULL; + size_t text_size = 0; + avro_value_t decoded; + avro_reader_t reader; + + /* A well-formed 3-byte string "abc": length 3 (zig-zag 6 -> 0x06), + * then the bytes 'a','b','c'. This must still decode. */ + const char valid[] = { 0x06, 'a', 'b', 'c' }; + + if (avro_schema_from_json_literal("\"string\"", &schema) != 0) { + fprintf(stderr, "valid: failed to parse schema: %s\n", avro_strerror()); + return EXIT_FAILURE; + } + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "valid: failed to create iface\n"); + avro_schema_decref(schema); + return EXIT_FAILURE; + } + + if (avro_generic_value_new(iface, &decoded) != 0) { + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return EXIT_FAILURE; + } + reader = avro_reader_memory(valid, sizeof(valid)); + rc = avro_value_read(reader, &decoded); + if (rc == 0 && avro_value_get_string(&decoded, &text, &text_size) == 0 && + text_size == 4 && strcmp(text, "abc") == 0) { + fprintf(stderr, "valid: well-formed string decoded as expected\n"); + ret = EXIT_SUCCESS; + } else { + fprintf(stderr, "valid: FAIL - well-formed string did not decode (rc=%d)\n", rc); + } + + avro_reader_free(reader); + avro_value_decref(&decoded); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return ret; +} + +/* An array of nulls: null elements occupy zero bytes, so a large declared + * count is legitimate and must not be rejected. */ +static int check_accepts_null_array(void) +{ + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + avro_value_t decoded; + avro_reader_t reader; + int rc; + int ret = EXIT_FAILURE; + size_t count = 0; + + /* count 127 (zig-zag 254 -> 0xFE 0x01), then the end-of-array marker 0. */ + const char null_array[] = { (char) 0xFE, (char) 0x01, 0x00 }; + + if (avro_schema_from_json_length("{\"type\":\"array\",\"items\":\"null\"}", + strlen("{\"type\":\"array\",\"items\":\"null\"}"), + &schema) != 0) { + fprintf(stderr, "null-array: failed to parse schema: %s\n", avro_strerror()); + return EXIT_FAILURE; + } + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + avro_schema_decref(schema); + return EXIT_FAILURE; + } + if (avro_generic_value_new(iface, &decoded) != 0) { + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return EXIT_FAILURE; + } + reader = avro_reader_memory(null_array, sizeof(null_array)); + rc = avro_value_read(reader, &decoded); + if (rc == 0 && avro_value_get_size(&decoded, &count) == 0 && count == 127) { + fprintf(stderr, "null-array: 127 nulls decoded, not falsely rejected\n"); + ret = EXIT_SUCCESS; + } else { + fprintf(stderr, "null-array: FAIL (rc=%d count=%zu): %s\n", + rc, count, avro_strerror()); + } + + avro_reader_free(reader); + avro_value_decref(&decoded); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return ret; +} + +int main(void) +{ + if (check_rejects_oversized("\"string\"", "string") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (check_rejects_oversized("\"bytes\"", "bytes") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + /* An array/map declaring 127 elements with no data must be rejected: the + * count exceeds the bytes that could back it. */ + if (check_rejects_oversized("{\"type\":\"array\",\"items\":\"long\"}", "array") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (check_rejects_oversized("{\"type\":\"map\",\"values\":\"long\"}", "map") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (check_accepts_valid() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (check_accepts_null_array() != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + return EXIT_SUCCESS; +} From 896a688ecf9a64d5876b2fa259f8e72fb7db8c39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 00:16:18 +0200 Subject: [PATCH 02/16] AVRO-4293: [c] Keep collection check enabled for deep schemas; assert EINVAL Review feedback: - min_bytes_per_element() returned 0 when the depth guard tripped, which made ensure_collection_available() skip the check and could bypass the guard for a crafted deep/recursive schema. Return 1 instead so the check stays enabled; a valid recursive value always encodes to at least 1 byte. - The test now includes and asserts the rejection code is EINVAL (the availability checks' error) rather than accepting any non-zero return, which would also pass on the pre-fix ENOSPC-after-allocation behavior. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 8 +++++++- lang/c/tests/test_avro_4293.c | 7 +++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index b44152d466e..813d46bdfe0 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -49,9 +49,15 @@ read_value(avro_reader_t reader, avro_value_t *dest); static int64_t min_bytes_per_element(avro_schema_t schema, int depth) { - if (schema == NULL || depth > 64) { + if (schema == NULL) { return 0; } + if (depth > 64) { + /* A cyclic or pathologically deep schema. Return 1 (not 0) so the + * collection check stays enabled rather than being silently + * bypassed; a valid recursive value always encodes to >= 1 byte. */ + return 1; + } switch (avro_typeof(schema)) { case AVRO_NULL: return 0; diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index c079a6071b8..e2310fcd70e 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -28,6 +28,7 @@ #include #include #include +#include #include /* Decodes buf against the given schema and returns the avro_value_read rc. @@ -89,6 +90,12 @@ static int check_rejects_oversized(const char *schema_literal, const char *label rc = try_decode(iface, oversized, sizeof(oversized)); if (rc == 0) { fprintf(stderr, "%s: FAIL - oversized length was accepted\n", label); + } else if (rc != EINVAL) { + /* The availability checks reject with EINVAL before allocating. + * A different error (e.g. ENOSPC from failing to read after a + * large allocation) would indicate the pre-fix behavior. */ + fprintf(stderr, "%s: FAIL - rejected with rc=%d (expected EINVAL): %s\n", + label, rc, avro_strerror()); } else { fprintf(stderr, "%s: oversized length rejected as expected: %s\n", label, avro_strerror()); From b549013089a8520eeddd101aa505ee69fe2a9f5c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 00:57:25 +0200 Subject: [PATCH 03/16] AVRO-4293: [c] Saturate min-bytes arithmetic; null-check reader in tests Review feedback: - min_bytes_per_element() now saturates the record field-minima sum to INT64_MAX instead of overflowing (a wrapped negative total would disable the collection check), and the map computation saturates the +1 for the key. - The valid-string and null-array tests now check avro_reader_memory() for NULL before use, so an allocation failure cannot dereference a NULL reader. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 17 ++++++++++++++--- lang/c/tests/test_avro_4293.c | 14 ++++++++++++++ 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 813d46bdfe0..a64c300c31e 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "avro/allocation.h" #include "avro/basics.h" @@ -74,7 +75,13 @@ min_bytes_per_element(avro_schema_t schema, int depth) for (i = 0; i < n; i++) { avro_schema_t field = avro_schema_record_field_get_by_index(schema, i); - total += min_bytes_per_element(field, depth + 1); + int64_t field_min = min_bytes_per_element(field, depth + 1); + /* Saturate rather than overflow: a wrapped (negative) + * total would disable the collection check. */ + if (field_min > INT64_MAX - total) { + return INT64_MAX; + } + total += field_min; } return total; } @@ -167,9 +174,13 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) int64_t min_bytes; avro_schema_t map_schema = avro_value_get_schema(dest); - /* Map keys are strings (>= 1 byte length prefix) plus the value. */ - min_bytes = 1 + min_bytes_per_element( + /* Map keys are strings (>= 1 byte length prefix) plus the value. + * Saturate the +1 so a maxed-out value minimum cannot wrap. */ + min_bytes = min_bytes_per_element( map_schema ? avro_schema_map_values(map_schema) : NULL, 0); + if (min_bytes < INT64_MAX) { + min_bytes += 1; + } check_prefix(rval, avro_binary_encoding.read_long(reader, &block_count), "Cannot read map block count: "); diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index e2310fcd70e..74054977fce 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -139,6 +139,13 @@ static int check_accepts_valid(void) return EXIT_FAILURE; } reader = avro_reader_memory(valid, sizeof(valid)); + if (reader == NULL) { + fprintf(stderr, "valid: avro_reader_memory failed\n"); + avro_value_decref(&decoded); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return EXIT_FAILURE; + } rc = avro_value_read(reader, &decoded); if (rc == 0 && avro_value_get_string(&decoded, &text, &text_size) == 0 && text_size == 4 && strcmp(text, "abc") == 0) { @@ -187,6 +194,13 @@ static int check_accepts_null_array(void) return EXIT_FAILURE; } reader = avro_reader_memory(null_array, sizeof(null_array)); + if (reader == NULL) { + fprintf(stderr, "null-array: avro_reader_memory failed\n"); + avro_value_decref(&decoded); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return EXIT_FAILURE; + } rc = avro_value_read(reader, &decoded); if (rc == 0 && avro_value_get_size(&decoded, &count) == 0 && count == 127) { fprintf(stderr, "null-array: 127 nulls decoded, not falsely rejected\n"); From 3601d0afd570f53bcb6a8644e5b7087e384b0f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 07:14:24 +0200 Subject: [PATCH 04/16] AVRO-4293: [c] Apply depth guard only to records so deep null returns 0 Review feedback (consistency with the C++ fix): the depth cutoff was applied before checking the schema type, so a zero-byte leaf type (e.g. null) nested under deeply chained records returned 1 instead of 0, enabling the collection check and potentially rejecting valid data. Move the depth guard into the AVRO_RECORD case; leaf types now return their true minimum (null -> 0) regardless of nesting depth. A cyclic link still terminates because it resolves through a record, which is depth-guarded. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index a64c300c31e..638f8383cc2 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -53,12 +53,6 @@ min_bytes_per_element(avro_schema_t schema, int depth) if (schema == NULL) { return 0; } - if (depth > 64) { - /* A cyclic or pathologically deep schema. Return 1 (not 0) so the - * collection check stays enabled rather than being silently - * bypassed; a valid recursive value always encodes to >= 1 byte. */ - return 1; - } switch (avro_typeof(schema)) { case AVRO_NULL: return 0; @@ -72,6 +66,14 @@ min_bytes_per_element(avro_schema_t schema, int depth) size_t n = avro_schema_record_size(schema); int64_t total = 0; size_t i; + if (depth > 64) { + /* A cyclic or pathologically deep record. Return 1 (not + * 0) so the collection check stays enabled; a valid + * recursive value always encodes to >= 1 byte. The depth + * guard is applied only here, so zero-byte leaf types + * such as null still return 0 regardless of depth. */ + return 1; + } for (i = 0; i < n; i++) { avro_schema_t field = avro_schema_record_field_get_by_index(schema, i); From d780e7701bae31fff643f4bb6d379a14d8e607bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 13:57:14 +0200 Subject: [PATCH 05/16] AVRO-4293: [c] Cap zero-byte collection allocation; fix map NULL-deref Completes the available-bytes protection for collections and supersedes the separate collection-limit change. Elements whose schema encodes to zero bytes (null, a zero-length fixed, or a record with only zero-byte fields) consume no input, so the bytes-remaining check cannot bound their count. A tiny payload declaring a huge array block count of such elements (e.g. {"type":"array","items":"null"} with a count of 200,000,000) therefore drove an unbounded avro_raw_array growth and exhausted memory. ensure_collection_available now enforces the bytes-remaining check plus a zero-byte cap (AVRO_DEFAULT_MAX_COLLECTION_ITEMS = 10,000,000) and a structural cap on all collections (AVRO_DEFAULT_MAX_COLLECTION_STRUCTURAL = Integer.MAX_VALUE - 8) covering readers that cannot report bytes remaining. AVRO_MAX_COLLECTION_ITEMS, when set, caps both. The limits and a min_bytes_per_element helper are exposed via avro_private.h so the datum skip path (datum_skip.c skip_array/skip_map) is bounded the same way, preventing an unbounded skip loop. Also fix a latent NULL-dereference in avro_raw_map_get_or_create: the result of avro_raw_array_append was dereferenced before the NULL check; reorder so the check happens first, returning ENOMEM cleanly on allocation failure. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/avro_private.h | 5 + lang/c/src/datum_skip.c | 30 +++++ lang/c/src/map.c | 9 +- lang/c/src/value-read.c | 99 +++++++++++++--- lang/c/tests/test_avro_4293.c | 215 ++++++++++++++++++++++++++++++++++ 5 files changed, 340 insertions(+), 18 deletions(-) diff --git a/lang/c/src/avro_private.h b/lang/c/src/avro_private.h index f97ef6b5a71..0c71c6962c1 100644 --- a/lang/c/src/avro_private.h +++ b/lang/c/src/avro_private.h @@ -27,6 +27,7 @@ extern "C" { #include "avro/errors.h" #include "avro/platform.h" +#include "avro/schema.h" #ifdef HAVE_CONFIG_H /* This is only true for now in the autotools build */ @@ -95,5 +96,9 @@ extern "C" { #define nullstrcmp(s1, s2) \ (((s1) && (s2)) ? strcmp(s1, s2) : ((s1) || (s2))) +/* Collection allocation limits, shared between the read and skip paths. */ +int64_t avro_min_bytes_per_element(avro_schema_t schema); +void avro_collection_limits(int64_t *zero_byte, int64_t *structural); + CLOSE_EXTERN #endif diff --git a/lang/c/src/datum_skip.c b/lang/c/src/datum_skip.c index e0ce561642e..c9ca74743c3 100644 --- a/lang/c/src/datum_skip.c +++ b/lang/c/src/datum_skip.c @@ -17,6 +17,7 @@ #include "avro_private.h" #include "avro/errors.h" +#include #include #include #include @@ -30,6 +31,16 @@ static int skip_array(avro_reader_t reader, const avro_encoding_t * enc, int64_t i; int64_t block_count; int64_t block_size; + int64_t skipped = 0; + int64_t zero_byte, structural, limit; + + /* Zero-byte elements skip with no per-item input, so bound them tightly; + * other elements consume bytes (bounded by EOF) so only the structural + * cap applies. Skipping allocates nothing, but an unbounded per-item loop + * is a CPU exhaustion. */ + avro_collection_limits(&zero_byte, &structural); + limit = (avro_min_bytes_per_element(writers_schema->items) > 0) + ? structural : zero_byte; check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read array block count: "); @@ -41,6 +52,13 @@ static int skip_array(avro_reader_t reader, const avro_encoding_t * enc, "Cannot read array block size: "); } + if (block_count > limit || skipped > limit - block_count) { + avro_set_error("Cannot skip a collection of more than %" + PRId64 " elements", limit); + return EINVAL; + } + skipped += block_count; + for (i = 0; i < block_count; i++) { check_prefix(rval, avro_skip_data(reader, writers_schema->items), "Cannot skip array element: "); @@ -57,6 +75,12 @@ static int skip_map(avro_reader_t reader, const avro_encoding_t * enc, { int rval; int64_t i, block_count; + int64_t skipped = 0; + int64_t zero_byte, structural; + + /* Map entries always carry a >= 1 byte key, so the structural cap applies + * (they are never zero-byte). */ + avro_collection_limits(&zero_byte, &structural); check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read map block count: "); @@ -67,6 +91,12 @@ static int skip_map(avro_reader_t reader, const avro_encoding_t * enc, check_prefix(rval, enc->read_long(reader, &block_size), "Cannot read map block size: "); } + if (block_count > structural || skipped > structural - block_count) { + avro_set_error("Cannot skip a collection of more than %" + PRId64 " elements", structural); + return EINVAL; + } + skipped += block_count; for (i = 0; i < block_count; i++) { check_prefix(rval, enc->skip_string(reader), "Cannot skip map key: "); diff --git a/lang/c/src/map.c b/lang/c/src/map.c index c85ffbd8493..5c166ad4294 100644 --- a/lang/c/src/map.c +++ b/lang/c/src/map.c @@ -109,13 +109,14 @@ int avro_raw_map_get_or_create(avro_raw_map_t *map, const char *key, i = map->elements.element_count; avro_raw_map_entry_t *raw_entry = (avro_raw_map_entry_t *) avro_raw_array_append(&map->elements); - raw_entry->key = avro_strdup(key); - st_insert((st_table *) map->indices_by_key, - (st_data_t) raw_entry->key, (st_data_t) i); if (!raw_entry) { - avro_str_free((char*)raw_entry->key); + /* avro_raw_array_append returns NULL on allocation + * failure; check before dereferencing it. */ return -ENOMEM; } + raw_entry->key = avro_strdup(key); + st_insert((st_table *) map->indices_by_key, + (st_data_t) raw_entry->key, (st_data_t) i); el = ((char *) raw_entry) + sizeof(avro_raw_map_entry_t); is_new = 1; } diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 638f8383cc2..5b336f61763 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -97,25 +97,96 @@ min_bytes_per_element(avro_schema_t schema, int depth) } } +/* Non-static wrapper so the skip path (datum_skip.c) can compute the minimum + * on-wire size of a collection's element schema. */ +int64_t +avro_min_bytes_per_element(avro_schema_t schema) +{ + return min_bytes_per_element(schema, 0); +} + +/* + * Maximum number of zero-byte-encoded collection elements (e.g. an array of + * nulls) to allocate from a single decode. Such elements consume no input, so + * the bytes-remaining check cannot bound their count; without a cap a tiny + * payload can declare a huge block count and exhaust memory. Overridable via + * the AVRO_MAX_COLLECTION_ITEMS environment variable. + */ +#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS ((int64_t) 10000000) + /* - * Reject a collection (array or map) block whose declared element count could - * not be backed by the bytes actually remaining, before appending. Skipped when - * the per-element minimum is zero or when the reader cannot report how many - * bytes remain. The comparison avoids overflow by dividing. + * Structural cap on the number of elements in any array or map (an overflow / + * defense-in-depth guard), matching the historical Integer.MAX_VALUE - 8 limit. + * Non-zero-byte elements are also bounded by the bytes remaining. + */ +#define AVRO_DEFAULT_MAX_COLLECTION_STRUCTURAL ((int64_t) 2147483639) + +/* + * Populate the zero-byte and structural collection limits. The + * AVRO_MAX_COLLECTION_ITEMS environment variable, when a non-negative integer, + * caps both; otherwise the tighter zero-byte default and the structural default + * are used. Shared with the skip path (see avro_collection_limits). + */ +void +avro_collection_limits(int64_t *zero_byte, int64_t *structural) +{ + const char *env = getenv("AVRO_MAX_COLLECTION_ITEMS"); + if (env != NULL && *env != '\0') { + char *end; + long long value = strtoll(env, &end, 10); + if (*end == '\0' && value >= 0) { + *zero_byte = *structural = (int64_t) value; + return; + } + } + *zero_byte = AVRO_DEFAULT_MAX_COLLECTION_ITEMS; + *structural = AVRO_DEFAULT_MAX_COLLECTION_STRUCTURAL; +} + +/* + * Reject a collection (array or map) block that could not be backed by the + * input, before appending. + * + * For elements with a positive minimum on-wire size, the declared count is + * checked against the bytes remaining. For zero-byte elements (e.g. an array of + * nulls), which consume no input and so cannot be bounded by the bytes + * remaining, the cumulative count is checked against a configurable limit. */ static int -ensure_collection_available(avro_reader_t reader, int64_t count, int64_t min_bytes) +ensure_collection_available(avro_reader_t reader, int64_t existing, + int64_t count, int64_t min_bytes) { int64_t available; - if (count <= 0 || min_bytes <= 0) { + int64_t zero_byte, structural; + if (count <= 0) { + return 0; + } + avro_collection_limits(&zero_byte, &structural); + if (min_bytes > 0) { + available = avro_reader_bytes_available(reader); + if (available >= 0 && count > available / min_bytes) { + avro_set_error("Collection claims %" PRId64 + " elements with at least %" PRId64 + " bytes each, but only %" PRId64 " bytes available", + count, min_bytes, available); + return EINVAL; + } + /* Structural / overflow guard, also covering readers that cannot + * report the bytes remaining. Compare without adding so + * existing + count cannot overflow. */ + if (count > structural || existing > structural - count) { + avro_set_error("Cannot read a collection of more than %" PRId64 + " elements; raise AVRO_MAX_COLLECTION_ITEMS " + "if this is legitimate", structural); + return EINVAL; + } return 0; } - available = avro_reader_bytes_available(reader); - if (available >= 0 && count > available / min_bytes) { - avro_set_error("Collection claims %" PRId64 - " elements with at least %" PRId64 - " bytes each, but only %" PRId64 " bytes available", - count, min_bytes, available); + /* Compare without adding, so existing + count cannot overflow. */ + if (count > zero_byte || existing > zero_byte - count) { + avro_set_error("Cannot read a collection of more than %" PRId64 + " zero-byte elements; raise AVRO_MAX_COLLECTION_ITEMS " + "if this is legitimate", zero_byte); return EINVAL; } return 0; @@ -147,7 +218,7 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) "Cannot read array block size: "); } - check(rval, ensure_collection_available(reader, block_count, min_bytes)); + check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes)); for (i = 0; i < (size_t) block_count; i++, index++) { avro_value_t child; @@ -195,7 +266,7 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) "Cannot read map block size: "); } - check(rval, ensure_collection_available(reader, block_count, min_bytes)); + check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes)); for (i = 0; i < (size_t) block_count; i++, index++) { char *key; diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index 74054977fce..c9b109db3e0 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -217,8 +217,166 @@ static int check_accepts_null_array(void) return ret; } +/* + * Zero-byte-element collection allocation limit + */ + +/* Encode a long as an Avro zig-zag varint into buf; returns the byte count. */ +static size_t encode_long(int64_t n, char *buf) +{ + uint64_t z = ((uint64_t) n << 1) ^ (uint64_t) (n >> 63); + size_t i = 0; + do { + uint8_t b = z & 0x7F; + z >>= 7; + if (z) { + b |= 0x80; + } + buf[i++] = (char) b; + } while (z); + return i; +} + +/* Build one array block of `count` elements (a negative count is followed by a + * block byte-size), plus the end-of-array marker. Returns the total length. */ +static size_t build_null_array(int64_t count, int negative, char *buf) +{ + size_t n = 0; + if (negative) { + n += encode_long(-count, buf + n); + n += encode_long(0, buf + n); /* block byte-size */ + } else { + n += encode_long(count, buf + n); + } + n += encode_long(0, buf + n); /* end-of-array marker */ + return n; +} + +/* Decode `buf` against `schema_literal` and return the avro_value_read rc. */ +static int decode_schema(const char *schema_literal, const char *buf, size_t buf_size) +{ + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + int rc; + + if (avro_schema_from_json_length(schema_literal, strlen(schema_literal), + &schema) != 0) { + fprintf(stderr, "failed to parse schema: %s\n", avro_strerror()); + return -1; + } + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + avro_schema_decref(schema); + return -1; + } + rc = try_decode(iface, buf, buf_size); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return rc; +} + +static int check_null_collection_rejected(const char *schema_literal, int64_t count, + int negative, const char *label) +{ + char buf[32]; + size_t len = build_null_array(count, negative, buf); + int rc = decode_schema(schema_literal, buf, len); + if (rc == EINVAL) { + fprintf(stderr, "%s: rejected as expected: %s\n", label, avro_strerror()); + return EXIT_SUCCESS; + } + fprintf(stderr, "%s: FAIL - rc=%d (expected EINVAL)\n", label, rc); + return EXIT_FAILURE; +} + +static int check_null_array_accepted(int64_t count, const char *label) +{ + char buf[32]; + size_t len = build_null_array(count, 0, buf); + int rc = decode_schema("{\"type\":\"array\",\"items\":\"null\"}", buf, len); + if (rc == 0) { + fprintf(stderr, "%s: accepted as expected\n", label); + return EXIT_SUCCESS; + } + fprintf(stderr, "%s: FAIL - rc=%d (expected 0)\n", label, rc); + return EXIT_FAILURE; +} + +/* Two blocks of `each` null elements followed by the end marker. */ +static int check_null_array_cumulative_rejected(int64_t each, const char *label) +{ + char buf[48]; + size_t n = 0; + int rc; + n += encode_long(each, buf + n); + n += encode_long(each, buf + n); + n += encode_long(0, buf + n); + rc = decode_schema("{\"type\":\"array\",\"items\":\"null\"}", buf, n); + if (rc == EINVAL) { + fprintf(stderr, "%s: rejected as expected: %s\n", label, avro_strerror()); + return EXIT_SUCCESS; + } + fprintf(stderr, "%s: FAIL - rc=%d (expected EINVAL)\n", label, rc); + return EXIT_FAILURE; +} + +/* Skipping a zero-byte array/map is bounded the same way as reading it. */ +static int check_skip_null_collection_rejected(const char *schema_literal, int64_t count, + const char *label) +{ + avro_schema_t schema = NULL; + avro_reader_t reader; + char buf[32]; + size_t len; + int rc; + + if (avro_schema_from_json_length(schema_literal, strlen(schema_literal), + &schema) != 0) { + return EXIT_FAILURE; + } + len = build_null_array(count, 0, buf); + reader = avro_reader_memory(buf, len); + rc = avro_skip_data(reader, schema); + avro_reader_free(reader); + avro_schema_decref(schema); + if (rc == EINVAL) { + fprintf(stderr, "%s: skip rejected as expected\n", label); + return EXIT_SUCCESS; + } + fprintf(stderr, "%s: FAIL - skip rc=%d (expected EINVAL)\n", label, rc); + return EXIT_FAILURE; +} + +/* A backed non-zero-byte array that passes the bytes check is still bounded by + * the structural cap (exercised with a lowered limit). */ +static int check_structural_cap_rejected(const char *label) +{ + char buf[64]; + size_t n = 0; + int i; + int rc; + n += encode_long(10, buf + n); /* block count 10 */ + for (i = 0; i < 10; i++) { + n += encode_long(i, buf + n); /* 10 real longs (1 byte each) */ + } + n += encode_long(0, buf + n); /* end marker */ + rc = decode_schema("{\"type\":\"array\",\"items\":\"long\"}", buf, n); + if (rc == EINVAL) { + fprintf(stderr, "%s: rejected as expected: %s\n", label, avro_strerror()); + return EXIT_SUCCESS; + } + fprintf(stderr, "%s: FAIL - rc=%d (expected EINVAL)\n", label, rc); + return EXIT_FAILURE; +} + int main(void) { + const char *array_null = "{\"type\":\"array\",\"items\":\"null\"}"; + const char *record_null = + "{\"type\":\"array\",\"items\":" + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[{\"name\":\"n\",\"type\":\"null\"}]}}"; + const char *map_null = "{\"type\":\"map\",\"values\":\"null\"}"; + if (check_rejects_oversized("\"string\"", "string") != EXIT_SUCCESS) { return EXIT_FAILURE; } @@ -239,5 +397,62 @@ int main(void) if (check_accepts_null_array() != EXIT_SUCCESS) { return EXIT_FAILURE; } + + /* The reported exploit: 200,000,000 nulls must be rejected by the + * default limit before allocating. */ + if (check_null_collection_rejected(array_null, 200000000, 0, + "array 200M (default limit)") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + /* A record whose only field is null also encodes to zero bytes. */ + if (check_null_collection_rejected(record_null, 200000000, 0, + "array 200M") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + /* A negative block count is normalized and must still be bounded. */ + if (check_null_collection_rejected(array_null, 200000000, 1, + "array negative count") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + + /* With a lowered limit, boundary behavior: 1000 accepted, 1001 rejected. */ + setenv("AVRO_MAX_COLLECTION_ITEMS", "1000", 1); + if (check_null_array_accepted(1000, "array 1000 within limit") != EXIT_SUCCESS) { + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + return EXIT_FAILURE; + } + if (check_null_collection_rejected(array_null, 1001, 0, + "array 1001 over limit") != EXIT_SUCCESS) { + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + return EXIT_FAILURE; + } + if (check_null_array_cumulative_rejected(600, "array cumulative 600+600") != EXIT_SUCCESS) { + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + return EXIT_FAILURE; + } + /* Skipping a huge zero-byte array is bounded too. */ + if (check_skip_null_collection_rejected(array_null, 5000, + "skip array over limit") != EXIT_SUCCESS) { + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + return EXIT_FAILURE; + } + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + + /* A backed non-zero-byte array is bounded by the structural cap. */ + setenv("AVRO_MAX_COLLECTION_ITEMS", "5", 1); + if (check_structural_cap_rejected("array over structural cap") != EXIT_SUCCESS) { + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + return EXIT_FAILURE; + } + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + + /* A huge map is bounded by the available-bytes check (each entry + * carries a >= 1 byte key), rejected with EINVAL as well. */ + if (check_null_collection_rejected(map_null, 200000000, 0, + "map 200M (available bytes)") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + return EXIT_SUCCESS; } + From c603c964070d47d3ae276a88aca9dd2b0d7883be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 18:14:34 +0200 Subject: [PATCH 06/16] AVRO-4293: [c] Reject INT64_MIN array/map block count Folds in AVRO-4275. A negative block count was negated with block_count * -1; for INT64_MIN this is signed-integer overflow (undefined behavior, CWE-190). On common platforms the result stays negative, and the subsequent cast to size_t in the loop yields an enormous iteration count, driving unbounded allocation. The zig-zag encoding of INT64_MIN is a valid 10-byte varint, so this is reachable from malformed input. Reject INT64_MIN before negating and use -block_count elsewhere, across all three decoder paths: the value reader (value-read.c), the datum consumer (consume-binary.c), and the datum skipper (datum_skip.c). Adds test_avro_4275 covering INT64_MIN rejection and correct handling of valid negative block counts for both arrays and maps. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/consume-binary.c | 20 +- lang/c/src/datum_skip.c | 12 +- lang/c/src/value-read.c | 16 +- lang/c/tests/CMakeLists.txt | 1 + lang/c/tests/test_avro_4275.c | 381 ++++++++++++++++++++++++++++++++++ 5 files changed, 424 insertions(+), 6 deletions(-) create mode 100644 lang/c/tests/test_avro_4275.c diff --git a/lang/c/src/consume-binary.c b/lang/c/src/consume-binary.c index 5e1db20684f..058202e6f52 100644 --- a/lang/c/src/consume-binary.c +++ b/lang/c/src/consume-binary.c @@ -53,12 +53,16 @@ read_array(avro_reader_t reader, const avro_encoding_t * enc, check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read array block count: "); + if (block_count == INT64_MIN) { + avro_set_error("Invalid array block count"); + return EINVAL; + } check(rval, avro_consumer_call(consumer, array_start_block, 1, block_count, ud)); while (block_count != 0) { if (block_count < 0) { - block_count = block_count * -1; + block_count = -block_count; check_prefix(rval, enc->read_long(reader, &block_size), "Cannot read array block size: "); } @@ -77,6 +81,10 @@ read_array(avro_reader_t reader, const avro_encoding_t * enc, check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read array block count: "); + if (block_count == INT64_MIN) { + avro_set_error("Invalid array block count"); + return EINVAL; + } check(rval, avro_consumer_call(consumer, array_start_block, 0, block_count, ud)); } @@ -96,12 +104,16 @@ read_map(avro_reader_t reader, const avro_encoding_t * enc, check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read map block count: "); + if (block_count == INT64_MIN) { + avro_set_error("Invalid map block count"); + return EINVAL; + } check(rval, avro_consumer_call(consumer, map_start_block, 1, block_count, ud)); while (block_count != 0) { if (block_count < 0) { - block_count = block_count * -1; + block_count = -block_count; check_prefix(rval, enc->read_long(reader, &block_size), "Cannot read map block size: "); } @@ -135,6 +147,10 @@ read_map(avro_reader_t reader, const avro_encoding_t * enc, check_prefix(rval, enc->read_long(reader, &block_count), "Cannot read map block count: "); + if (block_count == INT64_MIN) { + avro_set_error("Invalid map block count"); + return EINVAL; + } check(rval, avro_consumer_call(consumer, map_start_block, 0, block_count, ud)); } diff --git a/lang/c/src/datum_skip.c b/lang/c/src/datum_skip.c index c9ca74743c3..3c9f687fc37 100644 --- a/lang/c/src/datum_skip.c +++ b/lang/c/src/datum_skip.c @@ -47,7 +47,11 @@ static int skip_array(avro_reader_t reader, const avro_encoding_t * enc, while (block_count != 0) { if (block_count < 0) { - block_count = block_count * -1; + if (block_count == INT64_MIN) { + avro_set_error("Invalid array block count"); + return EINVAL; + } + block_count = -block_count; check_prefix(rval, enc->read_long(reader, &block_size), "Cannot read array block size: "); } @@ -87,7 +91,11 @@ static int skip_map(avro_reader_t reader, const avro_encoding_t * enc, while (block_count != 0) { int64_t block_size; if (block_count < 0) { - block_count = block_count * -1; + if (block_count == INT64_MIN) { + avro_set_error("Invalid map block count"); + return EINVAL; + } + block_count = -block_count; check_prefix(rval, enc->read_long(reader, &block_size), "Cannot read map block size: "); } diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 5b336f61763..488ba4dcc4e 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -212,7 +212,13 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) while (block_count != 0) { if (block_count < 0) { - block_count = block_count * -1; + /* Reject INT64_MIN: its negation is not + * representable in int64_t (CWE-190). */ + if (block_count == INT64_MIN) { + avro_set_error("Invalid array block count"); + return EINVAL; + } + block_count = -block_count; check_prefix(rval, avro_binary_encoding. read_long(reader, &block_size), "Cannot read array block size: "); @@ -260,7 +266,13 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) while (block_count != 0) { if (block_count < 0) { - block_count = block_count * -1; + /* Reject INT64_MIN: its negation is not + * representable in int64_t (CWE-190). */ + if (block_count == INT64_MIN) { + avro_set_error("Invalid map block count"); + return EINVAL; + } + block_count = -block_count; check_prefix(rval, avro_binary_encoding. read_long(reader, &block_size), "Cannot read map block size: "); diff --git a/lang/c/tests/CMakeLists.txt b/lang/c/tests/CMakeLists.txt index d2d6a4d6408..50d76659b23 100644 --- a/lang/c/tests/CMakeLists.txt +++ b/lang/c/tests/CMakeLists.txt @@ -89,3 +89,4 @@ add_avro_test_checkmem(test_avro_1906) add_avro_test_checkmem(test_avro_1904) add_avro_test_checkmem(test_avro_4246) add_avro_test_checkmem(test_avro_4293) +add_avro_test_checkmem(test_avro_4275) diff --git a/lang/c/tests/test_avro_4275.c b/lang/c/tests/test_avro_4275.c new file mode 100644 index 00000000000..b4d4bf0b78e --- /dev/null +++ b/lang/c/tests/test_avro_4275.c @@ -0,0 +1,381 @@ +/* + * 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. + */ + +/** + * Regression test for INT64_MIN negation overflow in read_array_value() + * and read_map_value() (CWE-190). + * + * The Avro binary format encodes block counts as zigzag-encoded varints. + * A negative block count means the absolute value is the actual count, + * preceded by a byte-size field. When block_count == INT64_MIN, the + * negation overflows (undefined behavior in C). This test verifies that + * the decoder rejects such malformed input gracefully. + */ + +#include +#include +#include +#include +#include +#include + +/* + * Zigzag encoding of INT64_MIN is 0xFFFFFFFFFFFFFFFF, which encodes as + * the 10-byte varint: FF FF FF FF FF FF FF FF FF 01 + */ +static const char int64min_block_count[] = { + (char)0xFF, (char)0xFF, (char)0xFF, (char)0xFF, (char)0xFF, + (char)0xFF, (char)0xFF, (char)0xFF, (char)0xFF, 0x01 +}; + +/* + * A valid array with negative block count: [-3] means 3 items follow, + * preceded by the block byte-size. + * + * Layout: + * 05 = varint 5 = zigzag(-3) => block_count = -3 + * 06 = varint 6 = zigzag(3) => block_size = 3 bytes + * 14 28 3C = zigzag ints: 10, 20, 30 + * 00 = terminator (block_count = 0) + */ +static const char valid_neg_block_array[] = { + '\x05', /* block_count = -3 */ + '\x06', /* block_size = 3 */ + '\x14', '\x28', '\x3C', /* ints: 10, 20, 30 */ + '\x00' /* terminator */ +}; + +/* + * A valid map with negative block count: [-2] means 2 entries follow. + * + * Layout: + * 03 = varint 3 = zigzag(-2) => block_count = -2 + * 0C = varint 12 = zigzag(6) => block_size = 6 bytes + * 02 61 14 = key "a" (len=1, 'a'), value int 10 + * 02 62 28 = key "b" (len=1, 'b'), value int 20 + * 00 = terminator + */ +static const char valid_neg_block_map[] = { + '\x03', /* block_count = -2 */ + '\x0C', /* block_size = 6 */ + '\x02', '\x61', '\x14', /* key "a", value 10 */ + '\x02', '\x62', '\x28', /* key "b", value 20 */ + '\x00' /* terminator */ +}; + +static int test_array_int64min(void) +{ + int rc; + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + avro_value_t value; + avro_reader_t reader = NULL; + + rc = avro_schema_from_json_literal( + "{\"type\":\"array\",\"items\":\"int\"}", &schema); + if (rc != 0) { + fprintf(stderr, "Failed to parse array schema: %s\n", + avro_strerror()); + return 1; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "Failed to create array iface\n"); + avro_schema_decref(schema); + return 1; + } + + rc = avro_generic_value_new(iface, &value); + if (rc != 0) { + fprintf(stderr, "Failed to create array value: %s\n", + avro_strerror()); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + reader = avro_reader_memory(int64min_block_count, + sizeof(int64min_block_count)); + if (reader == NULL) { + fprintf(stderr, "Failed to create reader\n"); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + /* This MUST fail with EINVAL rather than looping unboundedly or + * triggering undefined behavior from negating INT64_MIN. */ + rc = avro_value_read(reader, &value); + if (rc != EINVAL) { + fprintf(stderr, + "FAIL: INT64_MIN array block count: expected EINVAL, " + "got rc=%d (%s)\n", rc, avro_strerror()); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + printf("PASS: INT64_MIN array block count rejected with EINVAL: %s\n", + avro_strerror()); + + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 0; +} + +static int test_map_int64min(void) +{ + int rc; + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + avro_value_t value; + avro_reader_t reader = NULL; + + rc = avro_schema_from_json_literal( + "{\"type\":\"map\",\"values\":\"int\"}", &schema); + if (rc != 0) { + fprintf(stderr, "Failed to parse map schema: %s\n", + avro_strerror()); + return 1; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "Failed to create map iface\n"); + avro_schema_decref(schema); + return 1; + } + + rc = avro_generic_value_new(iface, &value); + if (rc != 0) { + fprintf(stderr, "Failed to create map value: %s\n", + avro_strerror()); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + reader = avro_reader_memory(int64min_block_count, + sizeof(int64min_block_count)); + if (reader == NULL) { + fprintf(stderr, "Failed to create reader\n"); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + /* This MUST fail with EINVAL rather than looping unboundedly or + * triggering undefined behavior from negating INT64_MIN. */ + rc = avro_value_read(reader, &value); + if (rc != EINVAL) { + fprintf(stderr, + "FAIL: INT64_MIN map block count: expected EINVAL, " + "got rc=%d (%s)\n", rc, avro_strerror()); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + printf("PASS: INT64_MIN map block count rejected with EINVAL: %s\n", + avro_strerror()); + + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 0; +} + +static int test_valid_neg_block_array(void) +{ + int rc; + size_t count; + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + avro_value_t value; + avro_reader_t reader = NULL; + + rc = avro_schema_from_json_literal( + "{\"type\":\"array\",\"items\":\"int\"}", &schema); + if (rc != 0) { + fprintf(stderr, "Failed to parse array schema: %s\n", + avro_strerror()); + return 1; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "Failed to create array iface\n"); + avro_schema_decref(schema); + return 1; + } + + rc = avro_generic_value_new(iface, &value); + if (rc != 0) { + fprintf(stderr, "Failed to create array value: %s\n", + avro_strerror()); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + reader = avro_reader_memory(valid_neg_block_array, + sizeof(valid_neg_block_array)); + if (reader == NULL) { + fprintf(stderr, "Failed to create reader\n"); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + rc = avro_value_read(reader, &value); + if (rc != 0) { + fprintf(stderr, + "FAIL: valid negative block count array rejected: %s\n", + avro_strerror()); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + rc = avro_value_get_size(&value, &count); + if (rc != 0 || count != 3) { + fprintf(stderr, + "FAIL: expected 3 elements, got %zu (rc=%d)\n", + count, rc); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + printf("PASS: valid negative block count array decoded (%zu items)\n", + count); + + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 0; +} + +static int test_valid_neg_block_map(void) +{ + int rc; + size_t count; + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + avro_value_t value; + avro_reader_t reader = NULL; + + rc = avro_schema_from_json_literal( + "{\"type\":\"map\",\"values\":\"int\"}", &schema); + if (rc != 0) { + fprintf(stderr, "Failed to parse map schema: %s\n", + avro_strerror()); + return 1; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "Failed to create map iface\n"); + avro_schema_decref(schema); + return 1; + } + + rc = avro_generic_value_new(iface, &value); + if (rc != 0) { + fprintf(stderr, "Failed to create map value: %s\n", + avro_strerror()); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + reader = avro_reader_memory(valid_neg_block_map, + sizeof(valid_neg_block_map)); + if (reader == NULL) { + fprintf(stderr, "Failed to create reader\n"); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + rc = avro_value_read(reader, &value); + if (rc != 0) { + fprintf(stderr, + "FAIL: valid negative block count map rejected: %s\n", + avro_strerror()); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + rc = avro_value_get_size(&value, &count); + if (rc != 0 || count != 2) { + fprintf(stderr, + "FAIL: expected 2 map entries, got %zu (rc=%d)\n", + count, rc); + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 1; + } + + printf("PASS: valid negative block count map decoded (%zu entries)\n", + count); + + avro_reader_free(reader); + avro_value_decref(&value); + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return 0; +} + +int main(void) +{ + int failures = 0; + + printf("Testing INT64_MIN block count rejection (CWE-190)...\n\n"); + + failures += test_array_int64min(); + failures += test_map_int64min(); + failures += test_valid_neg_block_array(); + failures += test_valid_neg_block_map(); + + printf("\n%s: %d test(s) failed\n", + failures ? "FAIL" : "OK", failures); + + return failures ? EXIT_FAILURE : EXIT_SUCCESS; +} From cdf28eeb02fcb729b5c5543778334737babd654a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 19:30:59 +0200 Subject: [PATCH 07/16] AVRO-4293: [c] Bound bytes/string allocation size; portable test env Addresses review feedback: - read_bytes/read_string bound the declared length to SIZE_MAX - 1 before computing the avro_malloc size, and do the +1 (NUL terminator) arithmetic in size_t. Previously a huge declared length could overflow *len + 1 when converted to size_t, undersizing the buffer and risking an out-of-bounds read/write (notably on 32-bit). - test_avro_4293: check_skip_null_collection_rejected() now checks avro_reader_memory() for NULL before use, and the collection-limit env var is set/unset through portable helpers (setenv/unsetenv, or _putenv_s under MSVC) so the test compiles on Windows toolchains. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/encoding_binary.c | 21 +++++++++++++++-- lang/c/tests/test_avro_4293.c | 44 ++++++++++++++++++++++++++++------- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/lang/c/src/encoding_binary.c b/lang/c/src/encoding_binary.c index f66200c9e13..912485bda20 100644 --- a/lang/c/src/encoding_binary.c +++ b/lang/c/src/encoding_binary.c @@ -21,6 +21,7 @@ #include "encoding.h" #include #include +#include #include #include @@ -143,7 +144,15 @@ static int read_bytes(avro_reader_t reader, char **bytes, int64_t * len) *len, available); return EINVAL; } - *bytes = (char *) avro_malloc(*len + 1); + /* Bound the length so the +1 (NUL terminator) cannot overflow the + * size_t allocation size, which could otherwise undersize the buffer + * and lead to an out-of-bounds read/write. */ + if ((uint64_t) *len > (uint64_t) (SIZE_MAX - 1)) { + avro_set_error("Bytes length %" PRId64 + " exceeds the maximum allocatable size", *len); + return EINVAL; + } + *bytes = (char *) avro_malloc((size_t) *len + 1); if (!*bytes) { avro_set_error("Cannot allocate buffer for bytes value"); return ENOMEM; @@ -211,8 +220,16 @@ static int read_string(avro_reader_t reader, char **s, int64_t *len) str_len, available); return EINVAL; } + /* Bound the length so the +1 (NUL terminator) cannot overflow the + * size_t allocation size, which could otherwise undersize the buffer + * and lead to an out-of-bounds read/write. */ + if ((uint64_t) str_len > (uint64_t) (SIZE_MAX - 1)) { + avro_set_error("String length %" PRId64 + " exceeds the maximum allocatable size", str_len); + return EINVAL; + } *len = str_len + 1; - *s = (char *) avro_malloc(*len); + *s = (char *) avro_malloc((size_t) str_len + 1); if (!*s) { avro_set_error("Cannot allocate buffer for string value"); return ENOMEM; diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index c9b109db3e0..9eeeb85ae2b 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -31,6 +31,28 @@ #include #include +/* Portable set/unset of the collection-limit environment variable: MSVC has no + * setenv/unsetenv, so use _putenv_s (an empty value unsets the variable). */ +#ifdef _WIN32 +static void set_collection_limit(const char *value) +{ + _putenv_s("AVRO_MAX_COLLECTION_ITEMS", value); +} +static void unset_collection_limit(void) +{ + _putenv_s("AVRO_MAX_COLLECTION_ITEMS", ""); +} +#else +static void set_collection_limit(const char *value) +{ + setenv("AVRO_MAX_COLLECTION_ITEMS", value, 1); +} +static void unset_collection_limit(void) +{ + unsetenv("AVRO_MAX_COLLECTION_ITEMS"); +} +#endif + /* Decodes buf against the given schema and returns the avro_value_read rc. * A non-zero rc means the read was rejected. */ static int try_decode(avro_value_iface_t *iface, const char *buf, size_t buf_size) @@ -336,6 +358,10 @@ static int check_skip_null_collection_rejected(const char *schema_literal, int64 } len = build_null_array(count, 0, buf); reader = avro_reader_memory(buf, len); + if (reader == NULL) { + avro_schema_decref(schema); + return EXIT_FAILURE; + } rc = avro_skip_data(reader, schema); avro_reader_free(reader); avro_schema_decref(schema); @@ -416,35 +442,35 @@ int main(void) } /* With a lowered limit, boundary behavior: 1000 accepted, 1001 rejected. */ - setenv("AVRO_MAX_COLLECTION_ITEMS", "1000", 1); + set_collection_limit("1000"); if (check_null_array_accepted(1000, "array 1000 within limit") != EXIT_SUCCESS) { - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); return EXIT_FAILURE; } if (check_null_collection_rejected(array_null, 1001, 0, "array 1001 over limit") != EXIT_SUCCESS) { - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); return EXIT_FAILURE; } if (check_null_array_cumulative_rejected(600, "array cumulative 600+600") != EXIT_SUCCESS) { - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); return EXIT_FAILURE; } /* Skipping a huge zero-byte array is bounded too. */ if (check_skip_null_collection_rejected(array_null, 5000, "skip array over limit") != EXIT_SUCCESS) { - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); return EXIT_FAILURE; } - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); /* A backed non-zero-byte array is bounded by the structural cap. */ - setenv("AVRO_MAX_COLLECTION_ITEMS", "5", 1); + set_collection_limit("5"); if (check_structural_cap_rejected("array over structural cap") != EXIT_SUCCESS) { - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); return EXIT_FAILURE; } - unsetenv("AVRO_MAX_COLLECTION_ITEMS"); + unset_collection_limit(); /* A huge map is bounded by the available-bytes check (each entry * carries a >= 1 byte key), rejected with EINVAL as well. */ From 36b71c2b08246c953b017d4d350ecdb25f1d4198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 20:35:56 +0200 Subject: [PATCH 08/16] AVRO-4293: [c] Handle map key allocation failure; fix test comment Addresses review feedback: - avro_raw_map_get_or_create duplicated the key with avro_strdup(), which memcpy()s into its buffer without a NULL check and so crashes on OOM. Copy the key via avro_str_alloc() with an explicit NULL check, index it only once the copy succeeds, and on failure roll back the just-appended element so the map is not left with a half-initialized (NULL-key) entry. - check_accepts_null_array: reword the comment so it no longer says a large null-array count "must not be rejected" (the zero-byte item cap does bound it); it verifies a moderate count is not rejected by the available-bytes check. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/map.c | 16 +++++++++++++++- lang/c/tests/test_avro_4293.c | 5 +++-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/lang/c/src/map.c b/lang/c/src/map.c index 5c166ad4294..062b30a1fa6 100644 --- a/lang/c/src/map.c +++ b/lang/c/src/map.c @@ -106,6 +106,8 @@ int avro_raw_map_get_or_create(avro_raw_map_t *map, const char *key, el = (char *) raw_entry + sizeof(avro_raw_map_entry_t); is_new = 0; } else { + char *key_copy; + size_t key_size; i = map->elements.element_count; avro_raw_map_entry_t *raw_entry = (avro_raw_map_entry_t *) avro_raw_array_append(&map->elements); @@ -114,7 +116,19 @@ int avro_raw_map_get_or_create(avro_raw_map_t *map, const char *key, * failure; check before dereferencing it. */ return -ENOMEM; } - raw_entry->key = avro_strdup(key); + /* Duplicate the key with a NULL-safe allocation: avro_strdup() + * memcpy()s into the buffer without checking it, so it would + * crash on OOM. Only index the entry once the copy succeeds; on + * failure, roll back the element we just appended so the map is + * not left with a half-initialized (NULL-key) entry. */ + key_size = strlen(key) + 1; + key_copy = avro_str_alloc(key_size); + if (!key_copy) { + map->elements.element_count--; + return -ENOMEM; + } + memcpy(key_copy, key, key_size); + raw_entry->key = key_copy; st_insert((st_table *) map->indices_by_key, (st_data_t) raw_entry->key, (st_data_t) i); el = ((char *) raw_entry) + sizeof(avro_raw_map_entry_t); diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index 9eeeb85ae2b..03b0ac2b5a4 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -184,8 +184,9 @@ static int check_accepts_valid(void) return ret; } -/* An array of nulls: null elements occupy zero bytes, so a large declared - * count is legitimate and must not be rejected. */ +/* An array of nulls within the configured limit: null elements occupy zero + * bytes, so a moderate declared count is legitimate and must not be rejected by + * the available-bytes check (it is instead bounded by the zero-byte item cap). */ static int check_accepts_null_array(void) { avro_schema_t schema = NULL; From 9b626d9dd9e60cb882697c31b7e015614b244470 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 21:04:55 +0200 Subject: [PATCH 09/16] AVRO-4293: [c] Depth guard returns 0; clamp collection limits Addresses review feedback: - min_bytes_per_element's depth guard now returns 0 instead of 1. Returning 1 misclassified a deeply-nested but zero-byte record (record-of-...-of-null) as non-zero-byte, bypassing the tighter zero-byte cap and falling back to the much larger structural cap for readers that cannot report bytes remaining. Returning 0 is a conservative lower bound that applies the zero-byte cap when the true minimum cannot be computed. - avro_collection_limits now clamps the parsed AVRO_MAX_COLLECTION_ITEMS to INT64_MAX and SIZE_MAX. The block count is cast to size_t in the read loops, so a configured limit above SIZE_MAX could permit a count that truncates on cast (notably on 32-bit) and weaken the bound. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 488ba4dcc4e..9ea210784a3 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -67,12 +67,13 @@ min_bytes_per_element(avro_schema_t schema, int depth) int64_t total = 0; size_t i; if (depth > 64) { - /* A cyclic or pathologically deep record. Return 1 (not - * 0) so the collection check stays enabled; a valid - * recursive value always encodes to >= 1 byte. The depth - * guard is applied only here, so zero-byte leaf types - * such as null still return 0 regardless of depth. */ - return 1; + /* Safety net for a pathologically deep (or cyclic) + * schema. Return 0 (a valid conservative lower bound) + * rather than over-estimating: if the true minimum + * cannot be computed, treat the element as possibly + * zero-byte so the tighter zero-byte collection cap is + * applied instead of the much larger structural cap. */ + return 0; } for (i = 0; i < n; i++) { avro_schema_t field = @@ -135,7 +136,18 @@ avro_collection_limits(int64_t *zero_byte, int64_t *structural) char *end; long long value = strtoll(env, &end, 10); if (*end == '\0' && value >= 0) { - *zero_byte = *structural = (int64_t) value; + /* Clamp to INT64_MAX and to SIZE_MAX: the block count is + * later cast to size_t in the read loop, so a limit above + * SIZE_MAX would permit a count that truncates on cast + * (notably on 32-bit) and weaken the bound. */ + uint64_t v = (uint64_t) value; + if (v > (uint64_t) INT64_MAX) { + v = (uint64_t) INT64_MAX; + } + if (v > (uint64_t) SIZE_MAX) { + v = (uint64_t) SIZE_MAX; + } + *zero_byte = *structural = (int64_t) v; return; } } From e333337df21c72af36f865bc210b40b98cfd8a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 21:36:53 +0200 Subject: [PATCH 10/16] AVRO-4293: [c] Bound read_string length to avoid int64_t overflow Addresses review feedback: *len = str_len + 1 can overflow when str_len == INT64_MAX (malformed input), which is undefined behavior. On 64-bit platforms SIZE_MAX > INT64_MAX, so the preceding SIZE_MAX - 1 check never fires for such a value. Add an explicit INT64_MAX - 1 bound before computing the +1. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/encoding_binary.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lang/c/src/encoding_binary.c b/lang/c/src/encoding_binary.c index 912485bda20..0b221b78e5d 100644 --- a/lang/c/src/encoding_binary.c +++ b/lang/c/src/encoding_binary.c @@ -220,10 +220,14 @@ static int read_string(avro_reader_t reader, char **s, int64_t *len) str_len, available); return EINVAL; } - /* Bound the length so the +1 (NUL terminator) cannot overflow the - * size_t allocation size, which could otherwise undersize the buffer - * and lead to an out-of-bounds read/write. */ - if ((uint64_t) str_len > (uint64_t) (SIZE_MAX - 1)) { + /* Bound the length so the +1 (NUL terminator) cannot overflow either + * the size_t allocation size (undersizing the buffer, leading to an + * out-of-bounds read/write) or the int64_t returned in *len. On 64-bit + * platforms SIZE_MAX > INT64_MAX, so the size_t check alone would let + * str_len == INT64_MAX through and make str_len + 1 overflow (undefined + * behavior); the INT64_MAX - 1 bound rejects it. */ + if ((uint64_t) str_len > (uint64_t) (SIZE_MAX - 1) + || str_len > INT64_MAX - 1) { avro_set_error("String length %" PRId64 " exceeds the maximum allocatable size", str_len); return EINVAL; From 889be2dbfa13dd30d6df453c4c77923df7bd076c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 23:14:38 +0200 Subject: [PATCH 11/16] AVRO-4293: [c] Bounds-check enum symbol index on read read_value's AVRO_ENUM case read the symbol index and passed it straight to avro_value_set_enum without validation, so a negative or >= symbol-count index was stored as an out-of-bounds ordinal (later resolved to a symbol name via an out-of-bounds read). Reject an index outside [0, symbol_count) with EINVAL, mirroring the existing union-discriminant check. Adds regression tests for a too-large and a negative enum index. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 10 +++++++ lang/c/tests/test_avro_4293.c | 56 +++++++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 9ea210784a3..287d69a05f1 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -527,9 +527,19 @@ read_value(avro_reader_t reader, avro_value_t *dest) case AVRO_ENUM: { int64_t val; + int symbol_count; + avro_schema_t enum_schema; check_prefix(rval, avro_binary_encoding. read_long(reader, &val), "Cannot read enum value: "); + enum_schema = avro_value_get_schema(dest); + symbol_count = + avro_schema_enum_number_of_symbols(enum_schema); + if (val < 0 || val >= symbol_count) { + avro_set_error("Invalid enum value: %" PRId64, + val); + return EINVAL; + } return avro_value_set_enum(dest, val); } diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index 03b0ac2b5a4..e6568061d0f 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -129,6 +129,45 @@ static int check_rejects_oversized(const char *schema_literal, const char *label return ret; } +/* Decodes an enum whose declared symbol index is out of range for the schema. + * The index must be rejected before it is stored, rather than accepted as an + * out-of-bounds ordinal. */ +static int check_enum_index_rejected(const char *encoded, size_t size, const char *label) +{ + const char *schema_literal = + "{\"type\":\"enum\",\"name\":\"E\",\"symbols\":[\"A\",\"B\"]}"; + avro_schema_t schema = NULL; + avro_value_iface_t *iface = NULL; + int rc; + int ret = EXIT_FAILURE; + + if (avro_schema_from_json_length(schema_literal, strlen(schema_literal), + &schema) != 0) { + fprintf(stderr, "%s: failed to parse schema: %s\n", label, avro_strerror()); + return EXIT_FAILURE; + } + + iface = avro_generic_class_from_schema(schema); + if (iface == NULL) { + fprintf(stderr, "%s: failed to create iface\n", label); + avro_schema_decref(schema); + return EXIT_FAILURE; + } + + rc = try_decode(iface, encoded, size); + if (rc == 0) { + fprintf(stderr, "%s: FAIL - out-of-range enum index was accepted\n", label); + } else { + fprintf(stderr, "%s: out-of-range enum index rejected as expected: %s\n", + label, avro_strerror()); + ret = EXIT_SUCCESS; + } + + avro_value_iface_decref(iface); + avro_schema_decref(schema); + return ret; +} + static int check_accepts_valid(void) { avro_schema_t schema = NULL; @@ -480,6 +519,23 @@ int main(void) return EXIT_FAILURE; } + /* An enum symbol index out of range (>= symbol count, or negative) must be + * rejected rather than stored as an out-of-bounds ordinal. The schema has + * two symbols; index 9 (zig-zag long -> 0x12) and index -1 (-> 0x01) are + * both invalid. */ + { + const char enum_too_large[] = { (char) 0x12 }; + const char enum_negative[] = { (char) 0x01 }; + if (check_enum_index_rejected(enum_too_large, sizeof(enum_too_large), + "enum index 9 of 2") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + if (check_enum_index_rejected(enum_negative, sizeof(enum_negative), + "enum index -1") != EXIT_SUCCESS) { + return EXIT_FAILURE; + } + } + return EXIT_SUCCESS; } From e06c25d34cb19641f1b0207622c89fba00a542e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 02:02:47 +0200 Subject: [PATCH 12/16] AVRO-4293: [c] Portable sign extension in the test's encode_long helper encode_long derived the zig-zag sign bits with (n >> 63) on a signed value, which is implementation-defined for negatives (arithmetic vs logical shift) and could produce wrong encodings on platforms that don't arithmetic-shift. Compute the all-ones/zero sign mask from a comparison instead. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/tests/test_avro_4293.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index e6568061d0f..28559fda465 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -286,7 +286,11 @@ static int check_accepts_null_array(void) /* Encode a long as an Avro zig-zag varint into buf; returns the byte count. */ static size_t encode_long(int64_t n, char *buf) { - uint64_t z = ((uint64_t) n << 1) ^ (uint64_t) (n >> 63); + /* Compute the sign extension portably: right-shifting a negative signed + * value is implementation-defined, so derive the all-ones/zero mask from + * a comparison instead of (n >> 63). */ + uint64_t sign = (n < 0) ? ~(uint64_t) 0 : (uint64_t) 0; + uint64_t z = ((uint64_t) n << 1) ^ sign; size_t i = 0; do { uint8_t b = z & 0x7F; From 7966926c172fcc673380931ee25c7017be3527f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 02:22:09 +0200 Subject: [PATCH 13/16] AVRO-4293: [c] Read collection limits once per decode, not per block ensure_collection_available called avro_collection_limits() (getenv + strtoll) on every block, adding avoidable overhead for many-block arrays/maps and letting the effective limit change mid-decode. Read the limits once at the start of read_array_value/read_map_value and pass zero_byte/structural into the check. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 287d69a05f1..5d1144da07b 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -166,14 +166,13 @@ avro_collection_limits(int64_t *zero_byte, int64_t *structural) */ static int ensure_collection_available(avro_reader_t reader, int64_t existing, - int64_t count, int64_t min_bytes) + int64_t count, int64_t min_bytes, + int64_t zero_byte, int64_t structural) { int64_t available; - int64_t zero_byte, structural; if (count <= 0) { return 0; } - avro_collection_limits(&zero_byte, &structural); if (min_bytes > 0) { available = avro_reader_bytes_available(reader); if (available >= 0 && count > available / min_bytes) { @@ -213,10 +212,15 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) int64_t block_count; int64_t block_size; int64_t min_bytes; + int64_t zero_byte, structural; avro_schema_t array_schema = avro_value_get_schema(dest); min_bytes = min_bytes_per_element( array_schema ? avro_schema_array_items(array_schema) : NULL, 0); + /* Read the limits once per decode rather than per block, so a + * many-block array does not re-parse AVRO_MAX_COLLECTION_ITEMS each + * time (and the effective limit is stable across the decode). */ + avro_collection_limits(&zero_byte, &structural); check_prefix(rval, avro_binary_encoding. read_long(reader, &block_count), @@ -236,7 +240,7 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) "Cannot read array block size: "); } - check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes)); + check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes, zero_byte, structural)); for (i = 0; i < (size_t) block_count; i++, index++) { avro_value_t child; @@ -263,6 +267,7 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) int64_t block_count; int64_t block_size; int64_t min_bytes; + int64_t zero_byte, structural; avro_schema_t map_schema = avro_value_get_schema(dest); /* Map keys are strings (>= 1 byte length prefix) plus the value. @@ -272,6 +277,8 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) if (min_bytes < INT64_MAX) { min_bytes += 1; } + /* Read the limits once per decode (see read_array_value). */ + avro_collection_limits(&zero_byte, &structural); check_prefix(rval, avro_binary_encoding.read_long(reader, &block_count), "Cannot read map block count: "); @@ -290,7 +297,7 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) "Cannot read map block size: "); } - check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes)); + check(rval, ensure_collection_available(reader, (int64_t) index, block_count, min_bytes, zero_byte, structural)); for (i = 0; i < (size_t) block_count; i++, index++) { char *key; From db7a9e1d6d4bf00d4d67dd06431bc37fbbe5f319 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 03:13:42 +0200 Subject: [PATCH 14/16] AVRO-4293: [c] Guard enum schema before the symbol-index range check read_value's AVRO_ENUM case called avro_schema_enum_number_of_symbols() on avro_value_get_schema(dest) without confirming it is an enum schema. A custom value interface returning NULL or a non-enum schema yields a negative error code, against which the range check would accept out-of-range indices. Reject a non-enum schema (is_avro_enum) up front. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 5d1144da07b..d732bc85ca4 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -540,6 +540,15 @@ read_value(avro_reader_t reader, avro_value_t *dest) read_long(reader, &val), "Cannot read enum value: "); enum_schema = avro_value_get_schema(dest); + /* A custom value interface could return a NULL or + * non-enum schema, for which + * avro_schema_enum_number_of_symbols returns a negative + * error code that would make the range check below accept + * out-of-range values. Reject that up front. */ + if (!is_avro_enum(enum_schema)) { + avro_set_error("Not an enum schema for enum value"); + return EINVAL; + } symbol_count = avro_schema_enum_number_of_symbols(enum_schema); if (val < 0 || val >= symbol_count) { From 3a0b1acfc1cb7f1f0d6760d1c62ffe0804af8d8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 03:31:56 +0200 Subject: [PATCH 15/16] AVRO-4293: [c] Bound read_bytes length to avoid int64_t len+1 overflow read_bytes bounded the length to SIZE_MAX-1 but, on 64-bit platforms where SIZE_MAX > INT64_MAX, still allowed len == INT64_MAX. The AVRO_BYTES caller in value-read.c then computes len + 1 (avro_wrapped_alloc_new), which overflows. Add the INT64_MAX - 1 bound, matching read_string. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/encoding_binary.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lang/c/src/encoding_binary.c b/lang/c/src/encoding_binary.c index 0b221b78e5d..e1abacaea04 100644 --- a/lang/c/src/encoding_binary.c +++ b/lang/c/src/encoding_binary.c @@ -144,10 +144,14 @@ static int read_bytes(avro_reader_t reader, char **bytes, int64_t * len) *len, available); return EINVAL; } - /* Bound the length so the +1 (NUL terminator) cannot overflow the - * size_t allocation size, which could otherwise undersize the buffer - * and lead to an out-of-bounds read/write. */ - if ((uint64_t) *len > (uint64_t) (SIZE_MAX - 1)) { + /* Bound the length so the +1 (NUL terminator) cannot overflow either + * the size_t allocation size here or the int64_t len + 1 computed by the + * AVRO_BYTES caller in value-read.c. On 64-bit platforms SIZE_MAX > + * INT64_MAX, so the size_t check alone would let len == INT64_MAX through + * and make len + 1 overflow (undefined behavior); the INT64_MAX - 1 bound + * rejects it. */ + if ((uint64_t) *len > (uint64_t) (SIZE_MAX - 1) + || *len > INT64_MAX - 1) { avro_set_error("Bytes length %" PRId64 " exceeds the maximum allocatable size", *len); return EINVAL; From 4ecd191f0dacb7c9281b2368c2f22fd2d672d61b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 06:45:06 +0200 Subject: [PATCH 16/16] AVRO-4293: [c] Clarify try_decode return-value contract in test comment try_decode() returns -1 on harness setup failures in addition to the avro_value_read rc. Document that so a -1 is not mistaken for a read rejection. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/tests/test_avro_4293.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lang/c/tests/test_avro_4293.c b/lang/c/tests/test_avro_4293.c index 28559fda465..4f6b5387144 100644 --- a/lang/c/tests/test_avro_4293.c +++ b/lang/c/tests/test_avro_4293.c @@ -53,8 +53,10 @@ static void unset_collection_limit(void) } #endif -/* Decodes buf against the given schema and returns the avro_value_read rc. - * A non-zero rc means the read was rejected. */ +/* Decodes buf against the given schema. Returns the avro_value_read rc (0 on a + * successful read, non-zero when the read is rejected), or -1 if the test + * harness itself fails to set up (avro_generic_value_new / avro_reader_memory). + * Callers distinguish "rejected" (> 0) from "harness error" (-1). */ static int try_decode(avro_value_iface_t *iface, const char *buf, size_t buf_size) { int rc;