Copilot commented on code in PR #3848:
URL: https://github.com/apache/avro/pull/3848#discussion_r3564158749


##########
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 
*/

Review Comment:
   The default max collection item limit constant is off by one: 2^31 - 8 is 
2147483640, but the code uses 2147483639. This changes the effective default 
limit and contradicts the comment.



##########
lang/c/src/value-read.c:
##########
@@ -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));

Review Comment:
   Same as arrays: negating a negative block count via multiplication can 
overflow for INT64_MIN, which is undefined behavior. Guard against INT64_MIN 
before negating and then perform the new collection size check using the 
normalized count.



##########
lang/c/src/value-read.c:
##########
@@ -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));

Review Comment:
   The negative-block path multiplies by -1 (`block_count = block_count * -1`), 
which is undefined behavior for INT64_MIN (hostile input can hit this). Add an 
explicit INT64_MIN guard before negating to avoid signed overflow UB, then call 
the new limit check with the normalized positive count.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to