From 7f3541d27323a53d2b7c9d22883e2c921afdb2fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 13:48:30 +0200 Subject: [PATCH 1/4] AVRO-4279: [c] Bound collection size when decoding arrays and maps The block count of an array or map is read from the input and drives allocation of the resulting collection. A very large or malformed block count (for example from truncated input) could request an unbounded allocation. Validate the block count per block and cumulatively against a configurable maximum (AVRO_MAX_COLLECTION_ITEMS), mirroring the Java SDK's collection item limit, and fail decoding with EINVAL when exceeded. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 48 +++++++++++++ lang/c/tests/CMakeLists.txt | 1 + lang/c/tests/test_avro_4279.c | 130 ++++++++++++++++++++++++++++++++++ 3 files changed, 179 insertions(+) create mode 100644 lang/c/tests/test_avro_4279.c diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index b6b6e79fadd..a1657dc4769 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -38,6 +38,50 @@ static int read_value(avro_reader_t reader, avro_value_t *dest); +/* + * The block count of an array or map is read from the (potentially untrusted or + * truncated) input and drives allocation of the resulting collection. To guard + * against unbounded memory allocation from a very large or malformed block + * count, the number of items in a single decoded array or map is capped. This + * mirrors the Java SDK's collection item limit. The default can be overridden + * with the AVRO_MAX_COLLECTION_ITEMS environment variable. + */ + +#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS ((int64_t) 2147483639) /* 2^31 - 8 */ + +static int64_t +avro_max_collection_items(void) +{ + const char *env = getenv("AVRO_MAX_COLLECTION_ITEMS"); + 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_COLLECTION_ITEMS; +} + +/* + * Ensure that decoding the next block of block_count items would not grow the + * collection (which already holds `existing` items) beyond the configured + * maximum. Returns 0 when it is safe to proceed, or EINVAL for a negative or + * over-large block count. + */ +static int +avro_check_collection_items(int64_t existing, int64_t block_count) +{ + int64_t limit = avro_max_collection_items(); + if (block_count < 0 || existing < 0 || block_count > limit - existing) { + avro_set_error("Cannot read collections larger than %lld items", + (long long) limit); + return EINVAL; + } + return 0; +} + + static int read_array_value(avro_reader_t reader, avro_value_t *dest) { @@ -59,6 +103,8 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) "Cannot read array block size: "); } + check(rval, avro_check_collection_items(index, block_count)); + for (i = 0; i < (size_t) block_count; i++, index++) { avro_value_t child; @@ -95,6 +141,8 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) "Cannot read map block size: "); } + check(rval, avro_check_collection_items(index, block_count)); + 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..d3ec8c6eca7 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_4279) diff --git a/lang/c/tests/test_avro_4279.c b/lang/c/tests/test_avro_4279.c new file mode 100644 index 00000000000..bf29b1b5533 --- /dev/null +++ b/lang/c/tests/test_avro_4279.c @@ -0,0 +1,130 @@ +/* + * 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-4279: the block count of an array or map is read from the input and + * drives allocation of the resulting collection. A very large or malformed + * block count must be rejected instead of attempting an unbounded allocation. + * The limit is configurable via the AVRO_MAX_COLLECTION_ITEMS environment + * variable, which these tests set to a small value. + */ + +#include +#include +#include +#include + +static avro_value_iface_t * +iface_for(const char *json) +{ + avro_schema_t schema = NULL; + if (avro_schema_from_json_length(json, strlen(json), &schema)) { + fprintf(stderr, "Cannot parse schema %s: %s\n", json, avro_strerror()); + exit(EXIT_FAILURE); + } + avro_value_iface_t *iface = avro_generic_class_from_schema(schema); + avro_schema_decref(schema); + if (iface == NULL) { + fprintf(stderr, "Cannot create value interface: %s\n", avro_strerror()); + exit(EXIT_FAILURE); + } + return iface; +} + +/* Attempt to decode `bytes` with the given interface. Returns the rc of the + * read (0 on success, non-zero on error). */ +static int +try_read(avro_value_iface_t *iface, const char *bytes, size_t len) +{ + avro_value_t value; + avro_generic_value_new(iface, &value); + avro_reader_t reader = avro_reader_memory(bytes, (int64_t) len); + int rc = avro_value_read(reader, &value); + avro_reader_free(reader); + avro_value_decref(&value); + return rc; +} + +static void +expect_rejected(avro_value_iface_t *iface, const char *bytes, size_t len, const char *label) +{ + if (try_read(iface, bytes, len) == 0) { + fprintf(stderr, "%s: expected read to be rejected but it succeeded\n", label); + exit(EXIT_FAILURE); + } +} + +static void +expect_accepted(avro_value_iface_t *iface, const char *bytes, size_t len, const char *label) +{ + if (try_read(iface, bytes, len) != 0) { + fprintf(stderr, "%s: expected read to succeed but it failed: %s\n", + label, avro_strerror()); + exit(EXIT_FAILURE); + } +} + +int main(void) +{ + /* Cap collections at 10 items for the duration of the test. */ +#ifdef _WIN32 + _putenv_s("AVRO_MAX_COLLECTION_ITEMS", "10"); +#else + setenv("AVRO_MAX_COLLECTION_ITEMS", "10", 1); +#endif + + avro_value_iface_t *array_iface = + iface_for("{\"type\": \"array\", \"items\": \"null\"}"); + avro_value_iface_t *map_iface = + iface_for("{\"type\": \"map\", \"values\": \"null\"}"); + + /* A single block declaring 11 items (zigzag(11) = 0x16) exceeds the + * configured limit of 10 and must be rejected. The null items occupy no + * bytes, so a tiny input could otherwise drive a large allocation. */ + { + const char bytes[] = { 0x16, 0x00 }; + expect_rejected(array_iface, bytes, sizeof(bytes), "array over limit"); + expect_rejected(map_iface, bytes, sizeof(bytes), "map over limit"); + } + + /* A negative count (unsigned varint 0x15 -> -11) uses its absolute value + * (11) as the item count and is followed by a block size (0x00). It must + * still be bounded. */ + { + const char bytes[] = { 0x15, 0x00 }; + expect_rejected(array_iface, bytes, sizeof(bytes), "array negative count"); + } + + /* Two blocks of 6 items (zigzag(6) = 0x0c) exceed the limit cumulatively + * even though neither block does on its own. */ + { + const char bytes[] = { 0x0c, 0x0c }; + expect_rejected(array_iface, bytes, sizeof(bytes), "array cumulative"); + } + + /* Three items (zigzag(3) = 0x06) then the end-of-array marker (0x00) is + * within the limit and must decode successfully. */ + { + const char bytes[] = { 0x06, 0x00 }; + expect_accepted(array_iface, bytes, sizeof(bytes), "array within limit"); + } + + avro_value_iface_decref(array_iface); + avro_value_iface_decref(map_iface); + + exit(EXIT_SUCCESS); +} From aa1d9eb0c76ab9417c9a8bcbc3e9c6a1797e9155 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 15:30:52 +0200 Subject: [PATCH 2/4] AVRO-4279: [c] Address review: guard INT64_MIN and fix comment - Reject a block count of INT64_MIN before negating it, avoiding signed integer overflow (undefined behavior) on hostile input, in both the array and map decoders. - Correct the default-limit comment: 2147483639 is Integer.MAX_VALUE - 8 (matching the Java SDK), not 2^31 - 8. - Add a test feeding an INT64_MIN block count. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 13 ++++++++++++- lang/c/tests/test_avro_4279.c | 13 +++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index a1657dc4769..360a72f5fdd 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -16,6 +16,7 @@ */ #include +#include #include #include @@ -47,7 +48,7 @@ read_value(avro_reader_t reader, avro_value_t *dest); * with the AVRO_MAX_COLLECTION_ITEMS environment variable. */ -#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS ((int64_t) 2147483639) /* 2^31 - 8 */ +#define AVRO_DEFAULT_MAX_COLLECTION_ITEMS ((int64_t) 2147483639) /* Integer.MAX_VALUE - 8, matching the Java SDK */ static int64_t avro_max_collection_items(void) @@ -97,6 +98,11 @@ read_array_value(avro_reader_t reader, avro_value_t *dest) while (block_count != 0) { if (block_count < 0) { + if (block_count == INT64_MIN) { + avro_set_error("Invalid array block count: %lld", + (long long) block_count); + return EINVAL; + } block_count = block_count * -1; check_prefix(rval, avro_binary_encoding. read_long(reader, &block_size), @@ -135,6 +141,11 @@ read_map_value(avro_reader_t reader, avro_value_t *dest) while (block_count != 0) { if (block_count < 0) { + if (block_count == INT64_MIN) { + avro_set_error("Invalid map block count: %lld", + (long long) block_count); + return EINVAL; + } block_count = block_count * -1; check_prefix(rval, avro_binary_encoding. read_long(reader, &block_size), diff --git a/lang/c/tests/test_avro_4279.c b/lang/c/tests/test_avro_4279.c index bf29b1b5533..c777f48f2c8 100644 --- a/lang/c/tests/test_avro_4279.c +++ b/lang/c/tests/test_avro_4279.c @@ -109,6 +109,19 @@ int main(void) expect_rejected(array_iface, bytes, sizeof(bytes), "array negative count"); } + /* A block count of INT64_MIN (zigzag of 2^64-1: nine 0xFF bytes then + * 0x01) cannot be negated without signed-overflow UB and must be rejected + * outright. */ + { + const char bytes[] = { + (char) 0xFF, (char) 0xFF, (char) 0xFF, (char) 0xFF, + (char) 0xFF, (char) 0xFF, (char) 0xFF, (char) 0xFF, + (char) 0xFF, 0x01 + }; + expect_rejected(array_iface, bytes, sizeof(bytes), "array INT64_MIN count"); + expect_rejected(map_iface, bytes, sizeof(bytes), "map INT64_MIN count"); + } + /* Two blocks of 6 items (zigzag(6) = 0x0c) exceed the limit cumulatively * even though neither block does on its own. */ { From d978884d15ce6ed87a9b2c4bba3413ec09e690e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 16:15:39 +0200 Subject: [PATCH 3/4] AVRO-4279: [c] Address review: strtoll overflow and test robustness - avro_max_collection_items now resets errno and rejects out-of-range values (strtoll returns LLONG_MAX/MIN with errno == ERANGE) and values above INT64_MAX; include and explicitly. - try_read now checks avro_generic_value_new and avro_reader_memory for failure instead of dereferencing a possibly-uninitialized value/NULL reader. - Assert the negative-block-count rejection for maps as well as arrays. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 11 +++++++++-- lang/c/tests/test_avro_4279.c | 13 +++++++++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index 360a72f5fdd..eece55dacb7 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -16,6 +16,8 @@ */ #include +#include +#include #include #include #include @@ -56,8 +58,13 @@ avro_max_collection_items(void) const char *env = getenv("AVRO_MAX_COLLECTION_ITEMS"); if (env != NULL && *env != '\0') { char *end = NULL; - long long value = strtoll(env, &end, 10); - if (end != NULL && *end == '\0' && value > 0) { + long long value; + errno = 0; + value = strtoll(env, &end, 10); + /* Reject trailing garbage and out-of-range values (strtoll returns + * LLONG_MAX/LLONG_MIN with errno == ERANGE on overflow). */ + if (errno == 0 && end != NULL && *end == '\0' && + value > 0 && value <= (long long) INT64_MAX) { return (int64_t) value; } } diff --git a/lang/c/tests/test_avro_4279.c b/lang/c/tests/test_avro_4279.c index c777f48f2c8..47f6cad8074 100644 --- a/lang/c/tests/test_avro_4279.c +++ b/lang/c/tests/test_avro_4279.c @@ -51,8 +51,16 @@ static int try_read(avro_value_iface_t *iface, const char *bytes, size_t len) { avro_value_t value; - avro_generic_value_new(iface, &value); + if (avro_generic_value_new(iface, &value) != 0) { + fprintf(stderr, "Cannot create value: %s\n", avro_strerror()); + exit(EXIT_FAILURE); + } avro_reader_t reader = avro_reader_memory(bytes, (int64_t) len); + if (reader == NULL) { + fprintf(stderr, "Cannot create memory reader\n"); + avro_value_decref(&value); + exit(EXIT_FAILURE); + } int rc = avro_value_read(reader, &value); avro_reader_free(reader); avro_value_decref(&value); @@ -103,10 +111,11 @@ int main(void) /* A negative count (unsigned varint 0x15 -> -11) uses its absolute value * (11) as the item count and is followed by a block size (0x00). It must - * still be bounded. */ + * still be bounded, for both arrays and maps. */ { const char bytes[] = { 0x15, 0x00 }; expect_rejected(array_iface, bytes, sizeof(bytes), "array negative count"); + expect_rejected(map_iface, bytes, sizeof(bytes), "map negative count"); } /* A block count of INT64_MIN (zigzag of 2^64-1: nine 0xFF bytes then From eb428958c640e7a7eb40cec9aa53626fc860364d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 16:32:16 +0200 Subject: [PATCH 4/4] AVRO-4279: [c] Address review: clamp limit to SIZE_MAX; map tests - Clamp a configured AVRO_MAX_COLLECTION_ITEMS to SIZE_MAX so the (size_t) cast of the block count in the decode loops cannot truncate on 32-bit platforms (no-op on 64-bit). - Add cumulative-limit and within-limit test cases for maps, mirroring the array cases, so regressions in the map path are caught. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/c/src/value-read.c | 7 +++++++ lang/c/tests/test_avro_4279.c | 23 +++++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/lang/c/src/value-read.c b/lang/c/src/value-read.c index eece55dacb7..f2917837be7 100644 --- a/lang/c/src/value-read.c +++ b/lang/c/src/value-read.c @@ -65,6 +65,13 @@ avro_max_collection_items(void) * LLONG_MAX/LLONG_MIN with errno == ERANGE on overflow). */ if (errno == 0 && end != NULL && *end == '\0' && value > 0 && value <= (long long) INT64_MAX) { + /* Clamp to SIZE_MAX so the (size_t) cast of the block count in + * the decode loops cannot truncate. On 64-bit builds size_t is + * at least as wide as int64_t so this never clamps; it only + * applies on 32-bit builds, where SIZE_MAX fits in int64_t. */ + if ((uint64_t) value > (uint64_t) SIZE_MAX) { + return (int64_t) SIZE_MAX; + } return (int64_t) value; } } diff --git a/lang/c/tests/test_avro_4279.c b/lang/c/tests/test_avro_4279.c index 47f6cad8074..cfb85706ca9 100644 --- a/lang/c/tests/test_avro_4279.c +++ b/lang/c/tests/test_avro_4279.c @@ -138,6 +138,18 @@ int main(void) expect_rejected(array_iface, bytes, sizeof(bytes), "array cumulative"); } + /* Same for maps: two blocks of 6 pairs (key string 0x02 0x61 = "a", + * null value = no bytes) exceed the limit cumulatively. */ + { + const char bytes[] = { + 0x0c, + 0x02, 0x61, 0x02, 0x61, 0x02, 0x61, + 0x02, 0x61, 0x02, 0x61, 0x02, 0x61, + 0x0c + }; + expect_rejected(map_iface, bytes, sizeof(bytes), "map cumulative"); + } + /* Three items (zigzag(3) = 0x06) then the end-of-array marker (0x00) is * within the limit and must decode successfully. */ { @@ -145,6 +157,17 @@ int main(void) expect_accepted(array_iface, bytes, sizeof(bytes), "array within limit"); } + /* A map of three pairs with distinct keys "a", "b", "c" then the + * end-of-map marker (0x00) is within the limit and must decode. */ + { + const char bytes[] = { + 0x06, + 0x02, 0x61, 0x02, 0x62, 0x02, 0x63, + 0x00 + }; + expect_accepted(map_iface, bytes, sizeof(bytes), "map within limit"); + } + avro_value_iface_decref(array_iface); avro_value_iface_decref(map_iface);