AVRO-4279: [c] Bound collection size when decoding arrays and maps

lang/c/src/value-read.c

@@ -16,6 +16,9 @@
  */
 
 #include <avro/platform.h>
+#include <errno.h>
+#include <limits.h>
+#include <stdint.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -38,6 +41,62 @@ 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)  /* Integer.MAX_VALUE - 8, matching the Java SDK */
+
+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;
+		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) {
+			/* 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;
+		}
+	}
+	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)
 {
@@ -53,12 +112,19 @@ 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),
 				     "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;
 
@@ -89,12 +155,19 @@ 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),
 				     "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;

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)

lang/c/tests/test_avro_4279.c

@@ -0,0 +1,175 @@
+/*
+ * 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 <avro.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+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;
+	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);
+	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, 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
+	 * 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. */
+	{
+		const char bytes[] = { 0x0c, 0x0c };
+		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. */
+	{
+		const char bytes[] = { 0x06, 0x00 };
+		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);
+
+	exit(EXIT_SUCCESS);
+}