Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3566581819
##########
lang/csharp/src/apache/main/IO/BinaryDecoder.cs:
##########
@@ -264,11 +264,70 @@ public void SkipFixed(int len)
// Read p bytes into a new byte buffer
private byte[] read(long p)
{
+ if (p < 0)
+ {
+ throw new AvroException($"Can not read a negative number of
bytes: {p}");
+ }
+
+ if (p > MaxDotNetArrayLength)
+ {
+ // A .NET array cannot be larger than this; reject with a
+ // consistent AvroException rather than letting new byte[p]
throw
+ // an OverflowException/OutOfMemoryException. Matches
ReadString().
+ throw new AvroException($"Length {p} exceeds the maximum
supported array length");
+ }
+
+ EnsureAvailableBytes(p);
byte[] buffer = new byte[p];
Read(buffer, 0, buffer.Length);
Review Comment:
`read(long p)` allocates `new byte[p]`, but array lengths in C# must be
`int`. This won’t compile (and even if it did, it would be unsafe). Since the
method already rejects `p > MaxDotNetArrayLength`, it’s safe to cast to int for
the allocation.
##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -501,6 +514,134 @@ 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 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 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);
+ private static readonly long MaxCollectionStructural =
ReadCollectionLimit(2147483639L);
+
+ private static long ReadCollectionLimit(long defaultValue)
+ {
+ string env =
Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS");
+ if (!string.IsNullOrEmpty(env) && long.TryParse(env, out long
value) && value >= 0)
+ {
+ return value;
+ }
+
+ return defaultValue;
+ }
Review Comment:
`AVRO_MAX_COLLECTION_ITEMS` is described as a cap, but
`ReadCollectionLimit()` currently replaces the default with any non-negative
value. If a higher value is set, `MaxCollectionStructural` can exceed
`int.MaxValue`, which reintroduces overflow risk when block counts are cast to
int (and contradicts the “cap” behavior). Consider treating the env var as an
upper bound (min with the default).
--
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]
