iemejia commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3565728544
##########
lang/csharp/src/apache/main/IO/BinaryDecoder.cs:
##########
@@ -264,11 +264,61 @@ 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);
return buffer;
}
+ /// <summary>
+ /// When the underlying stream can report its length, verifies that at
+ /// least <paramref name="length"/> bytes remain before the caller
+ /// allocates a buffer of that size. This guards against an
+ /// out-of-memory attack from a malicious or truncated input that
+ /// declares a huge length prefix but carries little actual data. The
+ /// check is skipped for non-seekable streams, whose remaining length
is
+ /// unknown.
+ /// </summary>
+ /// <param name="length">Number of bytes about to be read.</param>
+ internal void EnsureAvailableBytes(long length)
+ {
+ if (length > 0)
+ {
+ long remaining = RemainingBytes();
+ if (remaining >= 0 && length > remaining)
+ {
+ throw new AvroException(
+ $"Cannot read {length} bytes, only {remaining} bytes
remaining in the stream");
+ }
+ }
+ }
+
+ /// <summary>
+ /// Returns the number of bytes still available to read from the
+ /// underlying stream when it is seekable, or -1 when that count is not
+ /// known (a non-seekable stream). Used to reject a declared length or
a
+ /// collection block count that exceeds the data actually available
+ /// before allocating for it.
+ /// </summary>
+ /// <returns>The number of bytes remaining, or -1 if unknown.</returns>
+ public long RemainingBytes()
+ {
+ return stream.CanSeek ? stream.Length - stream.Position : -1;
+ }
Review Comment:
Fixed in 601f0ab0ae: RemainingBytes() now clamps a negative result (Position
past Length on a truncated or externally seeked stream) to 0, so callers only
ever see -1 (unknown) or a non-negative count.
##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -501,6 +511,101 @@ 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;
+ }
+
+ if (depth > 64)
+ {
+ // A cyclic or pathologically deep schema. Return 1 (not 0) so
the
+ // collection check stays enabled rather than being silently
+ // bypassed; a valid recursive value always encodes to >= 1
byte.
+ return 1;
+ }
+
+ 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:
+ // 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;
+ }
+ }
+
+ /// <summary>
+ /// Rejects a collection (array or map) block whose declared element
count
+ /// could not be backed by the bytes actually remaining, before
+ /// allocating. Skipped when the per-element minimum is zero, or when
the
+ /// decoder cannot report how many bytes remain.
+ /// </summary>
+ private static void EnsureCollectionAvailable(Decoder d, long count,
long minBytesPerElement)
+ {
+ if (count <= 0)
+ {
+ return;
+ }
+
Review Comment:
Fixed in 601f0ab0ae: EnsureCollectionAvailable now rejects a negative count
explicitly (it can arise from long.MinValue overflow when negating a negative
count) instead of returning early. The depth guard in MinBytesPerElement is
also applied only in the record case now, so a zero-byte leaf type (null) still
returns 0.
--
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]
