iemejia commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3566979852


##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -501,6 +514,141 @@ protected virtual object ReadMap(object reuse, MapSchema 
writerSchema, Schema re
             return result;
         }
 
+        /// <summary>
+        /// 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 encodes to zero bytes
+        /// returns 0 (not only <c>null</c>, but also composites that encode to
+        /// nothing, e.g. a record whose fields are all zero-byte), which 
disables
+        /// the bytes-remaining check for it (so an array of such elements is 
not
+        /// falsely rejected; they are instead bounded by the zero-byte item 
cap).
+        /// A depth limit breaks self-referencing schemas.
+        /// </summary>
+        private static int MinBytesPerElement(Schema schema, int depth = 0)
+        {
+            if (schema == null)
+            {
+                return 0;
+            }
+
+            switch (schema.Tag)
+            {
+                case Schema.Type.Null:
+                    return 0;
+                case Schema.Type.Float:
+                    return 4;
+                case Schema.Type.Double:
+                    return 8;
+                case Schema.Type.Fixed:
+                    return ((FixedSchema)schema).Size;
+                case Schema.Type.Record:
+                case Schema.Type.Error:
+                    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;
+                    }
+
+                    // Accumulate in a long and clamp so a deeply nested schema
+                    // cannot overflow int into a value <= 0, which would 
disable
+                    // the collection check.
+                    long total = 0;
+                    foreach (Field f in (RecordSchema)schema)
+                    {
+                        total += MinBytesPerElement(f.Schema, depth + 1);
+                        if (total >= int.MaxValue)
+                        {
+                            return int.MaxValue;
+                        }
+                    }
+
+                    return (int)total;
+                default:
+                    // boolean, int, long, bytes, string, enum, union, array, 
map:
+                    // all encode to at least one byte.
+                    return 1;
+            }
+        }
+
+        // Collection allocation limits, guarding against a block-count DoS. 
Both
+        // default to the same values as the other Avro SDKs and can be 
overridden
+        // (to a single value capping both) via the AVRO_MAX_COLLECTION_ITEMS
+        // environment variable.
+        private static readonly long MaxCollectionItems = 
ReadCollectionLimit(10_000_000L);
+        // The structural cap is additionally clamped to int.MaxValue: the 
callers
+        // cast the (cumulative) block count to int to size .NET collections, 
so a
+        // structural limit above int.MaxValue (e.g. from a large env override)
+        // would reintroduce an int-overflow on that cast.
+        private static readonly long MaxCollectionStructural =
+            Math.Min(ReadCollectionLimit(2147483639L), int.MaxValue);
+

Review Comment:
   Fixed in cf8a68752d. Added a per-TFM `MaxDotNetArrayLength` constant to 
GenericReader (0x7FFFFFC7 on modern .NET, 0x3FFFFFFF on netstandard2.0), 
mirroring BinaryDecoder, and clamped `MaxCollectionStructural` to it instead of 
`int.MaxValue` so a size passing EnsureCollectionAvailable can no longer fault 
inside Array.Resize.



-- 
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