From 2210a460a373f0fbf0c93cf5775a9bc682f61c53 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 22:27:36 +0200 Subject: [PATCH 01/21] AVRO-4295: [csharp] Validate available bytes before allocating for length-prefixed values and collections A bytes or string value is a length prefix followed by that many bytes, and an array or map block is an element count followed by that many items. A malicious or truncated input can declare a huge length or count with little or no data. - BinaryDecoder.RemainingBytes() reports the bytes still readable for a seekable stream (or -1). ReadBytes and both ReadString implementations reject an over-large declared length before allocating. - DefaultReader.ReadArray/ReadMap reject a block whose element count could not be backed by the bytes remaining, using MinBytesPerElement() computed from the element schema so a zero-byte element type (e.g. null) is not falsely rejected. The count is checked on the raw long before the int cast, which also avoids the cast overflowing into a bogus pre-allocation. Mirrors the Java SDK's checks (AVRO-4241). Non-seekable streams and non-binary decoders are unaffected. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 79 ++++++++++- .../src/apache/main/IO/BinaryDecoder.cs | 37 +++++ .../main/IO/BinaryDecoder.netstandard2.0.cs | 2 + .../IO/BinaryDecoder.notnetstandard2.0.cs | 2 + .../src/apache/test/IO/BinaryCodecTests.cs | 128 +++++++++++++++++- 5 files changed, 244 insertions(+), 4 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 0b945b9ff5e..cf7d50c8313 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -404,8 +404,14 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem ArraySchema rs = (ArraySchema)readerSchema; object result = CreateArray(reuse, rs); int i = 0; - for (int n = (int)d.ReadArrayStart(); n != 0; n = (int)d.ReadArrayNext()) + int minBytes = MinBytesPerElement(writerSchema.ItemSchema); + for (long nl = d.ReadArrayStart(); nl != 0; nl = d.ReadArrayNext()) { + // Reject a block whose element count could not be backed by the + // bytes remaining before allocating for it (checked on the raw + // long, which also avoids the int cast overflowing). + EnsureCollectionAvailable(d, nl, minBytes); + int n = (int)nl; if (GetArraySize(result) < (i + n)) ResizeArray(ref result, i + n); for (int j = 0; j < n; j++, i++) { @@ -490,8 +496,12 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re { MapSchema rs = (MapSchema)readerSchema; object result = CreateMap(reuse, rs); - for (int n = (int)d.ReadMapStart(); n != 0; n = (int)d.ReadMapNext()) + // Map keys are strings (>= 1 byte length prefix) plus the value. + int minBytes = 1 + MinBytesPerElement(writerSchema.ValueSchema); + for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { + EnsureCollectionAvailable(d, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++) { string k = d.ReadString(); @@ -501,6 +511,71 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re return result; } + /// + /// 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. + /// + private static int MinBytesPerElement(Schema schema, int depth = 0) + { + if (schema == null || depth > 64) + { + 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: + int total = 0; + foreach (Field f in (RecordSchema)schema) + { + total += MinBytesPerElement(f.Schema, depth + 1); + } + + return total; + default: + // boolean, int, long, bytes, string, enum, union, array, map: + // all encode to at least one byte. + return 1; + } + } + + /// + /// 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. + /// + private static void EnsureCollectionAvailable(Decoder d, long count, int minBytesPerElement) + { + if (count <= 0 || minBytesPerElement <= 0) + { + return; + } + + if (d is BinaryDecoder bd) + { + long remaining = bd.RemainingBytes(); + if (remaining >= 0 && count > remaining / minBytesPerElement) + { + throw new AvroException( + $"Collection claims {count} elements with at least {minBytesPerElement} bytes each, but only {remaining} bytes are available"); + } + } + } + /// /// Used by the default implementation of ReadMap() to create a fresh map object. The default /// implementation of this method returns a IDictionary<string, map>. diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 56aaa6e1815..1bbaf83853e 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -264,11 +264,48 @@ public void SkipFixed(int len) // Read p bytes into a new byte buffer private byte[] read(long p) { + EnsureAvailableBytes(p); byte[] buffer = new byte[p]; Read(buffer, 0, buffer.Length); return buffer; } + /// + /// When the underlying stream can report its length, verifies that at + /// least 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. + /// + /// Number of bytes about to be read. + 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"); + } + } + } + + /// + /// 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. + /// + /// The number of bytes remaining, or -1 if unknown. + public long RemainingBytes() + { + return stream.CanSeek ? stream.Length - stream.Position : -1; + } + private byte read() { int n = stream.ReadByte(); diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs index a37d6fa6c84..90ac362b13a 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs @@ -85,6 +85,8 @@ public string ReadString() throw new AvroException("Can not deserialize a string with negative length!"); } + EnsureAvailableBytes(length); + if (length > MaxDotNetArrayLength) { throw new AvroException("String length is not supported!"); diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs index c4a0dfaaf31..22cea838647 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs @@ -73,6 +73,8 @@ public string ReadString() throw new AvroException("Can not deserialize a string with negative length!"); } + EnsureAvailableBytes(length); + if (length <= MaxFastReadLength) { byte[] bufferArray = null; diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index a638b73fea2..8fb96aa6937 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -22,6 +22,7 @@ using System.Linq; using System.Text; using Avro.IO; +using Avro.Generic; namespace Avro.Test { @@ -278,10 +279,12 @@ public void TestInvalidInputWithMaxIntAsStringLength() iostr.Position = 0; Decoder d = new BinaryDecoder(iostr); + // The declared length far exceeds the bytes remaining in the + // (seekable) stream, so it is rejected before any allocation. var exception = Assert.Throws(() => d.ReadString()); Assert.NotNull(exception); - Assert.AreEqual("String length is not supported!", exception.Message); + StringAssert.Contains("bytes remaining in the stream", exception.Message); iostr.Close(); } } @@ -306,10 +309,12 @@ public void TestInvalidInputWithMaxArrayLengthAsStringLength() iostr.Position = 0; Decoder d = new BinaryDecoder(iostr); + // The declared length far exceeds the bytes remaining in the + // (seekable) stream, so it is rejected before any allocation. var exception = Assert.Throws(() => d.ReadString()); Assert.NotNull(exception); - Assert.AreEqual("Could not read as many bytes from stream as expected!", exception.Message); + StringAssert.Contains("bytes remaining in the stream", exception.Message); iostr.Close(); } } @@ -430,5 +435,124 @@ public void TestFixed(int size) TestSkip(b, (Decoder d) => d.SkipFixed(size), (Encoder e, byte[] t) => e.WriteFixed(t), size); } + + // A bytes/string value is a length prefix followed by that many bytes. + // A malicious or truncated input can declare a huge length with little + // actual data; on a seekable stream the reader must reject it before + // allocating, rather than attempting a huge allocation. + [Test] + public void TestReadBytesRejectsLengthBeyondStream() + { + MemoryStream ms = new MemoryStream(); + Encoder e = new BinaryEncoder(ms); + e.WriteLong(1_000_000); // declares 1,000,000 bytes... + ms.Position = 0; // ...but no data follows + Decoder d = new BinaryDecoder(ms); + Assert.Throws(() => d.ReadBytes()); + } + + [Test] + public void TestReadStringRejectsLengthBeyondStream() + { + MemoryStream ms = new MemoryStream(); + Encoder e = new BinaryEncoder(ms); + e.WriteLong(1_000_000); // declares 1,000,000 bytes... + ms.Position = 0; // ...but no data follows + Decoder d = new BinaryDecoder(ms); + Assert.Throws(() => d.ReadString()); + } + + // A well-formed value whose declared length fits the stream still reads. + [Test] + public void TestReadBytesWithinStreamStillReads() + { + byte[] payload = Encoding.UTF8.GetBytes("hello"); + MemoryStream ms = new MemoryStream(); + Encoder e = new BinaryEncoder(ms); + e.WriteBytes(payload); + ms.Position = 0; + Decoder d = new BinaryDecoder(ms); + Assert.AreEqual(payload, d.ReadBytes()); + } + + // On a non-seekable stream the remaining length is unknown, so the + // pre-check is skipped and a valid value still decodes. + [Test] + public void TestReadBytesNonSeekableStreamStillReads() + { + byte[] payload = Encoding.UTF8.GetBytes("hello"); + MemoryStream backing = new MemoryStream(); + Encoder e = new BinaryEncoder(backing); + e.WriteBytes(payload); + byte[] encoded = backing.ToArray(); + + using (var ns = new NonSeekableStream(new MemoryStream(encoded))) + { + Decoder d = new BinaryDecoder(ns); + Assert.AreEqual(payload, d.ReadBytes()); + } + } + + // An array/map block declares an element count; a malicious or truncated + // input can declare far more elements than the remaining bytes could + // hold. The count is validated against the bytes remaining before + // allocating, using the minimum on-wire size of the element schema (so + // 0-byte elements like null are not falsely rejected). + [Test] + public void TestReadArrayRejectsCountBeyondStream() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(1000000); // 1,000,000 longs, no data + ms.Position = 0; + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + [Test] + public void TestReadMapRejectsCountBeyondStream() + { + var schema = Avro.Schema.Parse("{\"type\":\"map\",\"values\":\"long\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(1000000); + ms.Position = 0; + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + [Test] + public void TestReadArrayOfNullNotFalselyRejected() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(100000); // one block of 100,000 nulls (zero bytes each) + enc.WriteLong(0); // end-of-array marker + ms.Position = 0; + var reader = new GenericReader(schema, schema); + var result = (Array)reader.Read(null, new BinaryDecoder(ms)); + Assert.AreEqual(100000, result.Length); + } + + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. + private sealed class NonSeekableStream : Stream + { + private readonly Stream inner; + public NonSeekableStream(Stream inner) { this.inner = inner; } + public override bool CanRead => true; + public override bool CanSeek => false; + public override bool CanWrite => false; + public override long Length => throw new NotSupportedException(); + public override long Position + { + get => throw new NotSupportedException(); + set => throw new NotSupportedException(); + } + public override void Flush() { } + public override int Read(byte[] buffer, int offset, int count) => inner.Read(buffer, offset, count); + public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); + public override void SetLength(long value) => throw new NotSupportedException(); + public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + } } } From 04308d0338897f54e1743986aa0f82be7f1d2ec0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 00:21:09 +0200 Subject: [PATCH 02/21] AVRO-4295: [csharp] Reject negative lengths and out-of-range counts; harden min-bytes sum Review feedback: - read(long) now rejects a negative length explicitly instead of letting it flow into a negative array allocation. - EnsureCollectionAvailable rejects a block count above int.MaxValue before the callers cast it to int, independently of the per-element size (so it also covers null-element arrays where the byte check is skipped). - MinBytesPerElement accumulates record field minima in a long and clamps to int.MaxValue, so deep nesting cannot overflow int into a value <= 0 that would disable the check; the depth guard now returns 1 (not 0) for the same reason. - The NonSeekableStream test helper disposes its wrapped inner stream. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 38 +++++++++++++++++-- .../src/apache/main/IO/BinaryDecoder.cs | 5 +++ .../src/apache/test/IO/BinaryCodecTests.cs | 36 ++++++++++++++++++ 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index cf7d50c8313..5a08c8d1e6e 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -521,11 +521,19 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re /// private static int MinBytesPerElement(Schema schema, int depth = 0) { - if (schema == null || depth > 64) + 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: @@ -538,13 +546,20 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) return ((FixedSchema)schema).Size; case Schema.Type.Record: case Schema.Type.Error: - int total = 0; + // 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 total; + return (int)total; default: // boolean, int, long, bytes, string, enum, union, array, map: // all encode to at least one byte. @@ -560,7 +575,22 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) /// private static void EnsureCollectionAvailable(Decoder d, long count, int minBytesPerElement) { - if (count <= 0 || minBytesPerElement <= 0) + if (count <= 0) + { + return; + } + + // A .NET collection cannot hold more than int.MaxValue elements and + // the callers cast the block count to int; reject an out-of-range + // count independently of the per-element size (which is 0 for e.g. + // arrays of null, where the byte check below is skipped). + if (count > int.MaxValue) + { + throw new AvroException( + $"Collection block count {count} exceeds the maximum supported size"); + } + + if (minBytesPerElement <= 0) { return; } diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 1bbaf83853e..978d3bbffc0 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -264,6 +264,11 @@ 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}"); + } + EnsureAvailableBytes(p); byte[] buffer = new byte[p]; Read(buffer, 0, buffer.Length); diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 8fb96aa6937..48b3de7990d 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -534,6 +534,32 @@ public void TestReadArrayOfNullNotFalselyRejected() Assert.AreEqual(100000, result.Length); } + // A negative bytes length must be rejected explicitly rather than + // flowing into a negative array allocation. + [Test] + public void TestReadBytesRejectsNegativeLength() + { + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(-5); + ms.Position = 0; + var d = new BinaryDecoder(ms); + Assert.Throws(() => d.ReadBytes()); + } + + // A block count larger than int.MaxValue must be rejected before the + // int cast, even for a null-element array where the byte check is + // skipped. + [Test] + public void TestReadArrayRejectsCountAboveIntMax() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong((long)int.MaxValue + 1); + ms.Position = 0; + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. private sealed class NonSeekableStream : Stream { @@ -553,6 +579,16 @@ public override void Flush() { } public override long Seek(long offset, SeekOrigin origin) => throw new NotSupportedException(); public override void SetLength(long value) => throw new NotSupportedException(); public override void Write(byte[] buffer, int offset, int count) => throw new NotSupportedException(); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + inner.Dispose(); + } + + base.Dispose(disposing); + } } } } From 7376905bbb6c2b6e7e3958299aaa07b41a037e98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 01:00:12 +0200 Subject: [PATCH 03/21] AVRO-4295: [csharp] Use long for collection min-bytes to avoid int overflow Review feedback: the map's minBytes was computed as 1 + MinBytesPerElement(...). Since MinBytesPerElement clamps to int.MaxValue, adding 1 in int arithmetic overflows to a negative value, which makes minBytesPerElement <= 0 and silently disables the remaining-bytes validation for maps. Compute both the array and map minima as long (1L + ...) and take a long in EnsureCollectionAvailable. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/Generic/GenericReader.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 5a08c8d1e6e..b89c0422582 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -404,7 +404,7 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem ArraySchema rs = (ArraySchema)readerSchema; object result = CreateArray(reuse, rs); int i = 0; - int minBytes = MinBytesPerElement(writerSchema.ItemSchema); + long minBytes = MinBytesPerElement(writerSchema.ItemSchema); for (long nl = d.ReadArrayStart(); nl != 0; nl = d.ReadArrayNext()) { // Reject a block whose element count could not be backed by the @@ -497,7 +497,7 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re MapSchema rs = (MapSchema)readerSchema; object result = CreateMap(reuse, rs); // Map keys are strings (>= 1 byte length prefix) plus the value. - int minBytes = 1 + MinBytesPerElement(writerSchema.ValueSchema); + long minBytes = 1L + MinBytesPerElement(writerSchema.ValueSchema); for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { EnsureCollectionAvailable(d, nl, minBytes); @@ -573,7 +573,7 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) /// allocating. Skipped when the per-element minimum is zero, or when the /// decoder cannot report how many bytes remain. /// - private static void EnsureCollectionAvailable(Decoder d, long count, int minBytesPerElement) + private static void EnsureCollectionAvailable(Decoder d, long count, long minBytesPerElement) { if (count <= 0) { From 4232c99984568e7cca8ba5c539dc174d9092ce97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 01:23:46 +0200 Subject: [PATCH 04/21] AVRO-4295: [csharp] Reject bytes length above the max .NET array length Review feedback: ReadBytes() -> read(long) allocates new byte[p]. A seekable stream can declare (and hold) more than the maximum .NET array length, so new byte[p] would throw an OverflowException/OutOfMemoryException instead of a consistent AvroException. Reject a length above MaxDotNetArrayLength first, as ReadString() already does. Added a test using a non-seekable stream. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/IO/BinaryDecoder.cs | 8 ++++++++ .../src/apache/test/IO/BinaryCodecTests.cs | 17 +++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 978d3bbffc0..11cd4080bb7 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -269,6 +269,14 @@ private byte[] read(long p) 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); diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 48b3de7990d..92c1f951ed3 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -560,6 +560,23 @@ public void TestReadArrayRejectsCountAboveIntMax() Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); } + // A bytes length above the maximum .NET array length must be rejected + // with a consistent AvroException rather than letting new byte[p] throw. + // A non-seekable stream is used so the length reaches the array-length + // check without needing gigabytes of backing data. + [Test] + public void TestReadBytesRejectsLengthAboveMaxArrayLength() + { + var backing = new MemoryStream(); + new BinaryEncoder(backing).WriteLong(3_000_000_000L); // > any MaxDotNetArrayLength + byte[] encoded = backing.ToArray(); + using (var ns = new NonSeekableStream(new MemoryStream(encoded))) + { + var d = new BinaryDecoder(ns); + Assert.Throws(() => d.ReadBytes()); + } + } + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. private sealed class NonSeekableStream : Stream { From 601f0ab0aee98c91f7ef5650bcef149cee89597a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 07:16:35 +0200 Subject: [PATCH 05/21] AVRO-4295: [csharp] Clamp RemainingBytes; reject negative counts; deep null returns 0 Review feedback: - RemainingBytes() 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 and the check is not skipped. - EnsureCollectionAvailable rejects a negative block count explicitly (it can arise from long.MinValue overflow when negating a negative count) instead of returning early and letting the caller cast it to int. - MinBytesPerElement applies the depth guard only in the record case, so a zero-byte leaf type (null) nested under deep records still returns 0. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 28 +++++++++++++------ .../src/apache/main/IO/BinaryDecoder.cs | 11 +++++++- 2 files changed, 29 insertions(+), 10 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index b89c0422582..95ce3dbcf80 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -526,14 +526,6 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) 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: @@ -546,6 +538,16 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) 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. @@ -575,11 +577,19 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) /// private static void EnsureCollectionAvailable(Decoder d, long count, long minBytesPerElement) { - if (count <= 0) + if (count == 0) { return; } + // A negative count is corrupt/malicious data (it can also arise from + // long.MinValue overflow when negating a negative block count), and + // the callers cast the block count to int; reject it explicitly. + if (count < 0) + { + throw new AvroException($"Invalid negative collection block count: {count}"); + } + // A .NET collection cannot hold more than int.MaxValue elements and // the callers cast the block count to int; reject an out-of-range // count independently of the per-element size (which is 0 for e.g. diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 11cd4080bb7..d2b0e29a27b 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -316,7 +316,16 @@ internal void EnsureAvailableBytes(long length) /// The number of bytes remaining, or -1 if unknown. public long RemainingBytes() { - return stream.CanSeek ? stream.Length - stream.Position : -1; + if (!stream.CanSeek) + { + return -1; + } + + // Clamp to 0: if the stream was externally seeked past its end (or + // truncated), Position can exceed Length. Callers should only ever + // see -1 (unknown) or a non-negative count. + long remaining = stream.Length - stream.Position; + return remaining < 0 ? 0 : remaining; } private byte read() From d200b5f073738188d66738b5a42cc31389682321 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 16:42:12 +0200 Subject: [PATCH 06/21] AVRO-4295: [csharp] Cap zero-byte collection element allocation Completes the available-bytes protection for collections. Elements whose schema encodes to zero bytes (null, a zero-length fixed, or a record with only zero-byte fields) consume no input, so the bytes-remaining check cannot bound their count. A tiny payload declaring a huge array block count of such elements (e.g. {"type":"array","items":"null"} with a count of 200,000,000) therefore drove an unbounded ResizeArray and exhausted memory. EnsureCollectionAvailable now tracks the cumulative count across blocks and enforces, per block: a structural cap on all collections (MaxCollectionStructural = Integer.MAX_VALUE - 8, also covering non-seekable decoders and keeping the total within the int range the callers cast to); a zero-byte item cap (MaxCollectionItems = 10,000,000) when the per-element minimum is zero; and the existing bytes-remaining check otherwise. AVRO_MAX_COLLECTION_ITEMS, when set, caps both. The reader array/map loops and the schema-resolution Skip path for arrays and maps are all bounded the same way, so skipping a huge zero-byte block cannot loop unboundedly. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 76 +++++++++++++------ .../src/apache/test/IO/BinaryCodecTests.cs | 34 +++++++++ 2 files changed, 88 insertions(+), 22 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 95ce3dbcf80..6ee2f7ca4fb 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -405,12 +405,14 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem object result = CreateArray(reuse, rs); int i = 0; long minBytes = MinBytesPerElement(writerSchema.ItemSchema); + long total = 0; for (long nl = d.ReadArrayStart(); nl != 0; nl = d.ReadArrayNext()) { // Reject a block whose element count could not be backed by the - // bytes remaining before allocating for it (checked on the raw - // long, which also avoids the int cast overflowing). - EnsureCollectionAvailable(d, nl, minBytes); + // bytes remaining (or, for zero-byte elements, that exceeds the + // item cap) before allocating for it. Checked on the raw long, + // which also avoids the int cast below overflowing. + total = EnsureCollectionAvailable(d, total, nl, minBytes); int n = (int)nl; if (GetArraySize(result) < (i + n)) ResizeArray(ref result, i + n); for (int j = 0; j < n; j++, i++) @@ -498,9 +500,10 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re object result = CreateMap(reuse, rs); // Map keys are strings (>= 1 byte length prefix) plus the value. long minBytes = 1L + MinBytesPerElement(writerSchema.ValueSchema); + long total = 0; for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { - EnsureCollectionAvailable(d, nl, minBytes); + total = EnsureCollectionAvailable(d, total, nl, minBytes); int n = (int)nl; for (int j = 0; j < n; j++) { @@ -569,19 +572,34 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) } } - /// - /// 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. - /// - private static void EnsureCollectionAvailable(Decoder d, long count, long minBytesPerElement) + // 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) { - if (count == 0) + string env = Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS"); + if (!string.IsNullOrEmpty(env) && long.TryParse(env, out long value) && value >= 0) { - return; + return value; } + return defaultValue; + } + + /// + /// Rejects a collection (array or map) block that could drive an unbounded + /// allocation, before allocating for it. A block whose declared element + /// count could not be backed by the bytes actually remaining is rejected; + /// zero-byte element blocks (where the bytes-remaining check does not + /// apply) are bounded by a cumulative item cap; and every collection is + /// bounded by a structural cap. Returns the running total across blocks. + /// + private static long EnsureCollectionAvailable(Decoder d, long total, long count, long minBytesPerElement) + { // A negative count is corrupt/malicious data (it can also arise from // long.MinValue overflow when negating a negative block count), and // the callers cast the block count to int; reject it explicitly. @@ -590,22 +608,28 @@ private static void EnsureCollectionAvailable(Decoder d, long count, long minByt throw new AvroException($"Invalid negative collection block count: {count}"); } - // A .NET collection cannot hold more than int.MaxValue elements and - // the callers cast the block count to int; reject an out-of-range - // count independently of the per-element size (which is 0 for e.g. - // arrays of null, where the byte check below is skipped). - if (count > int.MaxValue) + total += count; + + // A structural cap covers all collections, including non-seekable + // decoders that cannot report their remaining bytes, and keeps the + // running total within the int range the callers cast to. + if (total > MaxCollectionStructural) { throw new AvroException( - $"Collection block count {count} exceeds the maximum supported size"); + $"Collection size {total} exceeds the maximum allowed size of {MaxCollectionStructural}"); } if (minBytesPerElement <= 0) { - return; + // Zero-byte elements (e.g. null) consume no input, so the + // bytes-remaining check cannot bound them; cap by item count. + if (total > MaxCollectionItems) + { + throw new AvroException( + $"Collection of zero-byte elements ({total}) exceeds the maximum allowed size of {MaxCollectionItems}"); + } } - - if (d is BinaryDecoder bd) + else if (d is BinaryDecoder bd) { long remaining = bd.RemainingBytes(); if (remaining >= 0 && count > remaining / minBytesPerElement) @@ -614,6 +638,8 @@ private static void EnsureCollectionAvailable(Decoder d, long count, long minByt $"Collection claims {count} elements with at least {minBytesPerElement} bytes each, but only {remaining} bytes are available"); } } + + return total; } /// @@ -776,8 +802,11 @@ protected virtual void Skip(Schema writerSchema, Decoder d) case Schema.Type.Array: { Schema s = (writerSchema as ArraySchema).ItemSchema; + long minBytes = MinBytesPerElement(s); + long total = 0; for (long n = d.ReadArrayStart(); n != 0; n = d.ReadArrayNext()) { + total = EnsureCollectionAvailable(d, total, n, minBytes); for (long i = 0; i < n; i++) Skip(s, d); } } @@ -785,8 +814,11 @@ protected virtual void Skip(Schema writerSchema, Decoder d) case Schema.Type.Map: { Schema s = (writerSchema as MapSchema).ValueSchema; + long minBytes = 1L + MinBytesPerElement(s); + long total = 0; for (long n = d.ReadMapStart(); n != 0; n = d.ReadMapNext()) { + total = EnsureCollectionAvailable(d, total, n, minBytes); for (long i = 0; i < n; i++) { d.SkipString(); Skip(s, d); } } } diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 92c1f951ed3..090d4cc44bb 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -577,6 +577,40 @@ public void TestReadBytesRejectsLengthAboveMaxArrayLength() } } + // A zero-byte element type (null) consumes no input, so the + // bytes-remaining check cannot bound its block count; a tiny payload + // declaring a huge count must instead be rejected by the item cap + // before building the array. + [Test] + public void TestReadArrayOfNullRejectsHugeCount() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(200_000_000); // ~4 byte payload + ms.Position = 0; + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // The skip path (a writer field absent from the reader schema) must be + // bounded too, so skipping a huge zero-byte block cannot loop endlessly. + [Test] + public void TestSkipArrayOfNullRejectsHugeCount() + { + var writer = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"Foo\",\"fields\":[" + + "{\"name\":\"arr\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"val\",\"type\":\"int\"}]}"); + var reader = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"Foo\",\"fields\":[" + + "{\"name\":\"val\",\"type\":\"int\"}]}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(200_000_000); // arr block count; skipped + ms.Position = 0; + var r = new GenericReader(writer, reader); + Assert.Throws(() => r.Read(null, new BinaryDecoder(ms))); + } + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. private sealed class NonSeekableStream : Stream { From 99cefe0435b805c41d10ad2cf169b70420ad60c2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 18:16:37 +0200 Subject: [PATCH 07/21] AVRO-4295: [csharp] Reject long.MinValue array/map block count A negative block count is normalized by negating it (result = -result), but long.MinValue cannot be negated: under unchecked arithmetic it wraps back to a negative value, and under checked arithmetic it throws OverflowException. The zig-zag encoding of long.MinValue is a valid 10-byte varint, so this is reachable from malformed input. Reject long.MinValue explicitly in doReadItemCount() with a clear AvroException, consistent with the C and C++ bindings. Adds tests for a long.MinValue block count on ReadArrayStart() and ReadMapStart(). Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/IO/BinaryDecoder.cs | 8 ++++++++ .../src/apache/test/IO/BinaryCodecTests.cs | 19 +++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index d2b0e29a27b..a3d9d45c1d6 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -340,6 +340,14 @@ private long doReadItemCount() long result = ReadLong(); if (result < 0) { + // long.MinValue cannot be negated (it would overflow); reject it + // explicitly rather than propagating a wrapped negative or, under + // checked arithmetic, throwing an OverflowException. + if (result == long.MinValue) + { + throw new AvroException("Invalid negative block count: " + result); + } + ReadLong(); // Consume byte-count if present result = -result; } diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 090d4cc44bb..51115d26bd6 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -611,6 +611,25 @@ public void TestSkipArrayOfNullRejectsHugeCount() Assert.Throws(() => r.Read(null, new BinaryDecoder(ms))); } + // A block count of long.MinValue cannot be negated; it must be rejected + // rather than wrapping back to a negative (or huge) count. + [Test] + public void TestReadArrayRejectsInt64MinBlockCount() + { + // long.MinValue zig-zag encodes as the 10-byte varint below. + var data = new byte[] { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 }; + var d = new BinaryDecoder(new MemoryStream(data)); + Assert.Throws(() => d.ReadArrayStart()); + } + + [Test] + public void TestReadMapRejectsInt64MinBlockCount() + { + var data = new byte[] { 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01 }; + var d = new BinaryDecoder(new MemoryStream(data)); + Assert.Throws(() => d.ReadMapStart()); + } + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. private sealed class NonSeekableStream : Stream { From 3671989ce5e5ae9ab0852fa947a21e54f48066a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 19:01:38 +0200 Subject: [PATCH 08/21] AVRO-4295: [csharp] Cast bounded length to int; clamp structural cap Addresses review feedback: - read(long p) now allocates new byte[(int)p]; p is already bounded to <= MaxDotNetArrayLength above, so the cast required for array allocation cannot overflow. - MaxCollectionStructural is 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 AVRO_MAX_COLLECTION_ITEMS override) would otherwise reintroduce an int-overflow on that cast. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/Generic/GenericReader.cs | 7 ++++++- lang/csharp/src/apache/main/IO/BinaryDecoder.cs | 4 +++- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 6ee2f7ca4fb..3e5eb93bcd5 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -577,7 +577,12 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) // (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); + // 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); private static long ReadCollectionLimit(long defaultValue) { diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index a3d9d45c1d6..7e3ed03f1a7 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -278,7 +278,9 @@ private byte[] read(long p) } EnsureAvailableBytes(p); - byte[] buffer = new byte[p]; + // p has been bounded to <= MaxDotNetArrayLength above, so the cast to + // int (required for array allocation) cannot overflow. + byte[] buffer = new byte[(int)p]; Read(buffer, 0, buffer.Length); return buffer; } From 1af6c0a84fa0a6ae5b36b4b00b296b79d93e828a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 20:26:01 +0200 Subject: [PATCH 09/21] AVRO-4295: [csharp] Clarify two doc comments Addresses review feedback (documentation only): - read(long): the max-length guard "mirrors" ReadString() but the thrown message differs; reword so it no longer claims an exact match. - MinBytesPerElement summary: a zero-byte minimum is returned not only for null but also for composites that encode to zero bytes (e.g. a record whose fields are all zero-byte), which disables the bytes-remaining check (those are bounded by the zero-byte item cap instead). Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/Generic/GenericReader.cs | 10 ++++++---- lang/csharp/src/apache/main/IO/BinaryDecoder.cs | 3 ++- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 3e5eb93bcd5..83c9a61bfe0 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -517,10 +517,12 @@ protected virtual object ReadMap(object reuse, MapSchema writerSchema, Schema re /// /// 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. + /// backed by the bytes remaining. A type that encodes to zero bytes + /// returns 0 (not only null, 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. /// private static int MinBytesPerElement(Schema schema, int depth = 0) { diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 7e3ed03f1a7..9f2b7e6fbfd 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -273,7 +273,8 @@ private byte[] read(long p) { // 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(). + // an OverflowException/OutOfMemoryException, mirroring the + // maximum-length guard in ReadString() (the message differs). throw new AvroException($"Length {p} exceeds the maximum supported array length"); } From d3089624304f8e692768873f8dca908463ab98c9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 21:00:14 +0200 Subject: [PATCH 10/21] AVRO-4295: [csharp] Read string length as long to avoid int overflow Addresses review feedback: ReadString read the length prefix via ReadInt(), which casts the Avro long to int. A declared length above int.MaxValue overflowed to a negative int, bypassing EnsureAvailableBytes and throwing a misleading "negative length" error. Read the length as a long, apply the remaining-bytes guard and the max-array-length check, then cast to int. Applies to both the netstandard2.0 and non-netstandard2.0 decoders. Adds a regression test (via a non-seekable stream so the length check is reached). Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../main/IO/BinaryDecoder.netstandard2.0.cs | 12 ++++++-- .../IO/BinaryDecoder.notnetstandard2.0.cs | 30 +++++++++++-------- .../src/apache/test/IO/BinaryCodecTests.cs | 18 +++++++++++ 3 files changed, 45 insertions(+), 15 deletions(-) diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs index 90ac362b13a..2cb25cf5f37 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs @@ -78,7 +78,11 @@ public double ReadDouble() /// String read from the stream. public string ReadString() { - int length = ReadInt(); + // Read the length as a long: the prefix is an Avro long, so a value + // above int.MaxValue would overflow ReadInt() to a negative int, + // bypass EnsureAvailableBytes and throw a misleading "negative length" + // error. Validate the bounds before casting to int. + long length = ReadLong(); if (length < 0) { @@ -92,11 +96,13 @@ public string ReadString() throw new AvroException("String length is not supported!"); } + int intLength = (int)length; + using (var binaryReader = new BinaryReader(stream, Encoding.UTF8, true)) { - var bytes = binaryReader.ReadBytes(length); + var bytes = binaryReader.ReadBytes(intLength); - if (bytes.Length != length) + if (bytes.Length != intLength) { throw new AvroException("Could not read as many bytes from stream as expected!"); } diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs index 22cea838647..740de780c16 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.notnetstandard2.0.cs @@ -66,7 +66,11 @@ public double ReadDouble() /// String read from the stream. public string ReadString() { - int length = ReadInt(); + // Read the length as a long: the prefix is an Avro long, so a value + // above int.MaxValue would overflow ReadInt() to a negative int, + // bypass EnsureAvailableBytes and throw a misleading "negative length" + // error. Validate the bounds before casting to int. + long length = ReadLong(); if (length < 0) { @@ -75,15 +79,22 @@ public string ReadString() EnsureAvailableBytes(length); - if (length <= MaxFastReadLength) + if (length > MaxDotNetArrayLength) + { + throw new AvroException("String length is not supported!"); + } + + int intLength = (int)length; + + if (intLength <= MaxFastReadLength) { byte[] bufferArray = null; try { - Span buffer = length <= StackallocThreshold ? - stackalloc byte[length] : - (bufferArray = ArrayPool.Shared.Rent(length)).AsSpan(0, length); + Span buffer = intLength <= StackallocThreshold ? + stackalloc byte[intLength] : + (bufferArray = ArrayPool.Shared.Rent(intLength)).AsSpan(0, intLength); Read(buffer); @@ -99,16 +110,11 @@ public string ReadString() } else { - if (length > MaxDotNetArrayLength) - { - throw new AvroException("String length is not supported!"); - } - using (var binaryReader = new BinaryReader(stream, Encoding.UTF8, true)) { - var bytes = binaryReader.ReadBytes(length); + var bytes = binaryReader.ReadBytes(intLength); - if (bytes.Length != length) + if (bytes.Length != intLength) { throw new AvroException("Could not read as many bytes from stream as expected!"); } diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 51115d26bd6..1a764e4bf11 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -630,6 +630,24 @@ public void TestReadMapRejectsInt64MinBlockCount() Assert.Throws(() => d.ReadMapStart()); } + // A string length prefix above int.MaxValue must be rejected as an + // unsupported length, not overflow the int cast into a negative length. + // A non-seekable stream is used so the check is reached without the + // remaining-bytes guard firing first. + [Test] + public void TestReadStringRejectsLengthAboveInt32() + { + var backing = new MemoryStream(); + new BinaryEncoder(backing).WriteLong((long)int.MaxValue + 1); + byte[] encoded = backing.ToArray(); + using (var ns = new NonSeekableStream(new MemoryStream(encoded))) + { + var d = new BinaryDecoder(ns); + var ex = Assert.Throws(() => d.ReadString()); + StringAssert.Contains("not supported", ex.Message); + } + } + // Minimal read-only, forward-only stream wrapper reporting CanSeek=false. private sealed class NonSeekableStream : Stream { From cf8a68752d2e8e3ee9a494724367f78a4885be90 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 21:34:51 +0200 Subject: [PATCH 11/21] AVRO-4295: [csharp] Clamp structural cap to runtime max array length Addresses review feedback: MaxCollectionStructural was clamped to int.MaxValue, but the default reader grows its backing array via Array.Resize, whose maximum supported length is smaller (0x7FFFFFC7 on modern .NET, 0x3FFFFFFF on netstandard2.0). A collection size passing EnsureCollectionAvailable could still throw OutOfMemoryException/OverflowException inside ResizeArray instead of a deterministic AvroException. Clamp the structural limit to the runtime's max array length, mirroring BinaryDecoder.MaxDotNetArrayLength. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 23 +++++++++++++++---- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 83c9a61bfe0..a001f39089a 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -579,12 +579,25 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) // (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. + + // The largest array the runtime can allocate. Mirrors + // BinaryDecoder.MaxDotNetArrayLength: the default reader grows its backing + // array via Array.Resize, which throws (OutOfMemoryException/OverflowException) + // above this length rather than a deterministic AvroException. +#if NETSTANDARD2_0 + private const int MaxDotNetArrayLength = 0x3FFFFFFF; +#else + private const int MaxDotNetArrayLength = 0x7FFFFFC7; +#endif + + // The structural cap is additionally clamped to the runtime's maximum + // array length: the callers cast the (cumulative) block count to int to + // size .NET collections, and a limit above the max array length (e.g. from + // a large env override, or int.MaxValue itself) would let a collection + // that passes EnsureCollectionAvailable still fault inside Array.Resize + // instead of failing deterministically. private static readonly long MaxCollectionStructural = - Math.Min(ReadCollectionLimit(2147483639L), int.MaxValue); + Math.Min(ReadCollectionLimit(2147483639L), MaxDotNetArrayLength); private static long ReadCollectionLimit(long defaultValue) { From 20fb487ce19dfe138ac06e93f42099a6ee2f0e31 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 22:42:04 +0200 Subject: [PATCH 12/21] AVRO-4295: [csharp] Grow array on demand instead of preallocating count ReadArray resized the backing array to the full declared block count up front (ResizeArray(ref result, i + n)) before reading any element. On a non-seekable stream EnsureCollectionAvailable cannot bound the count against the remaining bytes, so a large declared count could drive a multi-gigabyte Array.Resize before the truncated input is detected. Preallocate only a bounded amount (MaxCollectionPrealloc) up front and grow the array geometrically on demand as elements are read. Blocks no larger than the cap keep the original single-resize fast path; a hostile count now fails with a bounded AvroException after a small allocation. Adds a non-seekable-stream regression test. (ReadMap already grows via Dictionary.Add, so it is unaffected.) Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 31 ++++++++++++++++++- .../src/apache/test/IO/BinaryCodecTests.cs | 20 ++++++++++++ 2 files changed, 50 insertions(+), 1 deletion(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index a001f39089a..6664efa1b52 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -414,9 +414,28 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem // which also avoids the int cast below overflowing. total = EnsureCollectionAvailable(d, total, nl, minBytes); int n = (int)nl; - if (GetArraySize(result) < (i + n)) ResizeArray(ref result, i + n); + // Preallocate only a bounded amount up front, then grow on demand + // below. On a non-seekable stream EnsureCollectionAvailable cannot + // bound the count, so resizing straight to i+n could allocate a + // huge array before any element is read; a truncated stream instead + // fails within Read() after a bounded growth. Blocks no larger than + // the cap keep the original single-resize fast path. + int prealloc = i + Math.Min(n, MaxCollectionPrealloc); + if (GetArraySize(result) < prealloc) ResizeArray(ref result, prealloc); for (int j = 0; j < n; j++, i++) { + if (GetArraySize(result) <= i) + { + int current = GetArraySize(result); + int grown = current + (current >> 1) + 1; + if (grown <= i) + { + grown = i + 1; + } + + ResizeArray(ref result, grown); + } + SetArrayElement(result, i, Read(GetArrayElement(result, i), writerSchema.ItemSchema, rs.ItemSchema, d)); } } @@ -599,6 +618,16 @@ private static int MinBytesPerElement(Schema schema, int depth = 0) private static readonly long MaxCollectionStructural = Math.Min(ReadCollectionLimit(2147483639L), MaxDotNetArrayLength); + // Upper bound on how many elements the backing array is grown by in a + // single step while decoding. The array still grows to hold every element + // actually read; this only avoids resizing to the full (possibly + // attacker-declared) block count up front, before any element is read. + // That matters most for non-seekable streams, where the bytes-available + // check cannot bound the declared count, so a single Array.Resize to the + // block count could allocate a huge array before the truncated stream is + // detected. + private const int MaxCollectionPrealloc = 1024; + private static long ReadCollectionLimit(long defaultValue) { string env = Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS"); diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 1a764e4bf11..da644f64004 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -592,6 +592,26 @@ public void TestReadArrayOfNullRejectsHugeCount() Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); } + // A non-zero-byte array on a non-seekable stream cannot have its block + // count bounded by the bytes remaining (the length is unknown). The + // backing array must therefore be grown on demand rather than + // preallocated to the declared count, so a huge count with truncated data + // fails with a bounded AvroException instead of attempting a multi- + // gigabyte allocation. + [Test] + public void TestReadArrayHugeCountOnStreamClampsPreallocation() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}"); + var backing = new MemoryStream(); + new BinaryEncoder(backing).WriteLong(200_000_000); // block count; no element data + byte[] encoded = backing.ToArray(); + using (var ns = new NonSeekableStream(new MemoryStream(encoded))) + { + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ns))); + } + } + // The skip path (a writer field absent from the reader schema) must be // bounded too, so skipping a huge zero-byte block cannot loop endlessly. [Test] From 6350b1ef5b21ffbe1b8e35a5a7644939205734d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 23:22:17 +0200 Subject: [PATCH 13/21] AVRO-4295: [csharp] Explicitly bounds-check union and enum indices UnionSchema's indexer performed no range check (an out-of-range or negative branch index surfaced as an ArgumentOutOfRangeException from the IList indexer), and EnumSchema's indexer checked only the upper bound (a negative symbol index fell through to Symbols[index] and threw ArgumentOutOfRangeException). Validate the index against [0, count) in both indexers and throw a clear AvroException, so a malformed union/enum index on read fails deterministically with an Avro-specific error. Adds tests for negative and too-large union and enum indices via GenericReader. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Schema/EnumSchema.cs | 4 +-- .../src/apache/main/Schema/UnionSchema.cs | 6 ++++ .../src/apache/test/IO/BinaryCodecTests.cs | 28 +++++++++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/lang/csharp/src/apache/main/Schema/EnumSchema.cs b/lang/csharp/src/apache/main/Schema/EnumSchema.cs index 225780310a6..6b3db1a16f9 100644 --- a/lang/csharp/src/apache/main/Schema/EnumSchema.cs +++ b/lang/csharp/src/apache/main/Schema/EnumSchema.cs @@ -231,8 +231,8 @@ public string this[int index] { get { - if (index < Symbols.Count) return Symbols[index]; - throw new AvroException("Enumeration out of range. Must be less than " + Symbols.Count + ", but is " + index); + if (index >= 0 && index < Symbols.Count) return Symbols[index]; + throw new AvroException("Enumeration out of range. Must be in [0, " + Symbols.Count + "), but is " + index); } } diff --git a/lang/csharp/src/apache/main/Schema/UnionSchema.cs b/lang/csharp/src/apache/main/Schema/UnionSchema.cs index af9ba758363..3f48d5c88ab 100644 --- a/lang/csharp/src/apache/main/Schema/UnionSchema.cs +++ b/lang/csharp/src/apache/main/Schema/UnionSchema.cs @@ -100,6 +100,12 @@ public Schema this[int index] { get { + if (index < 0 || index >= Schemas.Count) + { + throw new AvroException( + "Union branch index out of range. Must be in [0, " + Schemas.Count + "), but is " + index); + } + return Schemas[index]; } } diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index da644f64004..23c1c23a463 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -642,6 +642,34 @@ public void TestReadArrayRejectsInt64MinBlockCount() Assert.Throws(() => d.ReadArrayStart()); } + // A union branch index outside [0, branch count) must be rejected with a + // clear AvroException rather than an ArgumentOutOfRangeException. + [Test] + public void TestReadUnionRejectsOutOfRangeIndex() + { + var schema = Avro.Schema.Parse("[\"null\",\"long\"]"); + // Branch index 5 (zig-zag long 0x0a) and -1 (0x01); only 2 branches exist. + foreach (var data in new[] { new byte[] { 0x0a }, new byte[] { 0x01 } }) + { + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(new MemoryStream(data)))); + } + } + + // An enum symbol index outside [0, symbol count) must be rejected with a + // clear AvroException. + [Test] + public void TestReadEnumRejectsOutOfRangeIndex() + { + var schema = Avro.Schema.Parse("{\"type\":\"enum\",\"name\":\"E\",\"symbols\":[\"A\",\"B\"]}"); + // Symbol index 9 (zig-zag int 0x12) and -1 (0x01); only 2 symbols exist. + foreach (var data in new[] { new byte[] { 0x12 }, new byte[] { 0x01 } }) + { + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(new MemoryStream(data)))); + } + } + [Test] public void TestReadMapRejectsInt64MinBlockCount() { From 91609bd5fe0ac71395642c6ff9e3f312f70765dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 12 Jul 2026 23:49:40 +0200 Subject: [PATCH 14/21] AVRO-4295: [csharp] Reject overlong varints in ReadLong ReadLong looped over continuation bytes with no cap, so an overlong varint was accepted (with the shift wrapping mod 64) as a silently wrong value instead of being rejected. A 64-bit value uses at most 10 bytes; reject an 11th continuation byte with AvroException, matching the Java, C and C++ SDKs. Adds a regression test. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/IO/BinaryDecoder.cs | 7 +++++++ lang/csharp/src/apache/test/IO/BinaryCodecTests.cs | 13 +++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 9f2b7e6fbfd..3d56ab9219b 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -76,6 +76,13 @@ public long ReadLong() int shift = 7; while ((b & 0x80) != 0) { + // A 64-bit value uses at most 10 bytes (shifts 0..63); reject an + // overlong varint rather than silently wrapping to a wrong value. + if (shift >= 70) + { + throw new AvroException("Varint is too long"); + } + b = read(); n |= (b & 0x7FUL) << shift; shift += 7; diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 23c1c23a463..ed4aaabc2ac 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -678,6 +678,19 @@ public void TestReadMapRejectsInt64MinBlockCount() Assert.Throws(() => d.ReadMapStart()); } + // A 64-bit value uses at most 10 bytes; an 11th continuation byte is a + // malformed (overlong) varint and must be rejected rather than silently + // wrapping to a wrong value. + [Test] + public void TestReadLongRejectsOverlongVarint() + { + var data = new byte[11]; + for (int i = 0; i < 10; i++) data[i] = 0x80; // 10 continuation bytes + data[10] = 0x01; + var d = new BinaryDecoder(new MemoryStream(data)); + Assert.Throws(() => d.ReadLong()); + } + // A string length prefix above int.MaxValue must be rejected as an // unsupported length, not overflow the int cast into a negative length. // A non-seekable stream is used so the check is reached without the From 345a803a86f172745d5e6fd59ff02f6a55cb1c89 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 01:13:23 +0200 Subject: [PATCH 15/21] AVRO-4295: [csharp] Reject block-count overflow before adding to the total EnsureCollectionAvailable did `total += count` before the cap checks. With a collection split across blocks (e.g. block 1 count=1, block 2 count=long.MaxValue) the addition could overflow, wrapping `total` negative and bypassing both the structural cap and the zero-byte item cap, letting an oversized count reach the later (int) cast. Check `count > MaxCollectionStructural - total` before adding (total is always <= MaxCollectionStructural on entry and count >= 0, so the subtraction cannot underflow/overflow), rejecting the overflow deterministically. Adds a non-seekable-stream regression test. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 16 +++++++------- .../src/apache/test/IO/BinaryCodecTests.cs | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 6664efa1b52..bc5e9c4571d 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -657,17 +657,19 @@ private static long EnsureCollectionAvailable(Decoder d, long total, long count, throw new AvroException($"Invalid negative collection block count: {count}"); } - total += count; - - // A structural cap covers all collections, including non-seekable - // decoders that cannot report their remaining bytes, and keeps the - // running total within the int range the callers cast to. - if (total > MaxCollectionStructural) + // Reject before adding so an oversized block count cannot overflow + // `total` (wrapping it negative and bypassing the caps below). The + // running total is always <= MaxCollectionStructural on entry (the + // invariant this method maintains) and count >= 0, so the subtraction + // cannot underflow or overflow. + if (count > MaxCollectionStructural - total) { throw new AvroException( - $"Collection size {total} exceeds the maximum allowed size of {MaxCollectionStructural}"); + $"Collection block count {count} exceeds the maximum allowed size of {MaxCollectionStructural}"); } + total += count; + if (minBytesPerElement <= 0) { // Zero-byte elements (e.g. null) consume no input, so the diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index ed4aaabc2ac..a66aa80e761 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -631,6 +631,27 @@ public void TestSkipArrayOfNullRejectsHugeCount() Assert.Throws(() => r.Read(null, new BinaryDecoder(ms))); } + // A collection split across blocks must not let the running total + // overflow: block 1 count=1 then block 2 count=long.MaxValue would wrap + // the total negative and bypass the caps. A non-seekable stream disables + // the bytes-remaining check, so the structural pre-add check must reject it. + [Test] + public void TestReadArrayRejectsBlockCountOverflow() + { + var backing = new MemoryStream(); + var enc = new BinaryEncoder(backing); + enc.WriteLong(1); // block 1: one element + enc.WriteLong(42); // the element + enc.WriteLong(long.MaxValue); // block 2: huge count -> would overflow total + byte[] encoded = backing.ToArray(); + using (var ns = new NonSeekableStream(new MemoryStream(encoded))) + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"long\"}"); + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ns))); + } + } + // A block count of long.MinValue cannot be negated; it must be rejected // rather than wrapping back to a negative (or huge) count. [Test] From ffecc24c52e2a3b9f74934ed9f59bf94f03daaac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 01:34:39 +0200 Subject: [PATCH 16/21] AVRO-4295: [csharp] Clamp array growth to the structural cap The on-demand growth `current + (current >> 1) + 1` was computed in int (risking overflow to a negative size for very large arrays) and could overshoot the runtime's max array length / the structural cap, making Array.Resize throw a runtime exception even when the declared count was within limits. Compute the growth in long and clamp it to MaxCollectionStructural (which is <= the max .NET array length); the validated element count never exceeds that cap, so the clamp cannot starve a legitimate collection. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/main/Generic/GenericReader.cs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index bc5e9c4571d..fb1fac08283 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -427,13 +427,24 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem if (GetArraySize(result) <= i) { int current = GetArraySize(result); - int grown = current + (current >> 1) + 1; - if (grown <= i) + // Grow ~1.5x, computed in long to avoid int overflow, and + // clamp to the structural cap (which is <= the runtime's + // max array length). The validated element count never + // exceeds that cap, so clamping cannot starve a legitimate + // collection while it keeps Array.Resize from being handed + // an over-large (or overflowed/negative) size. + long grown = (long)current + (current >> 1) + 1; + if (grown < i + 1) { grown = i + 1; } - ResizeArray(ref result, grown); + if (grown > MaxCollectionStructural) + { + grown = MaxCollectionStructural; + } + + ResizeArray(ref result, (int)grown); } SetArrayElement(result, i, Read(GetArrayElement(result, i), writerSchema.ItemSchema, rs.ItemSchema, d)); From 8361138aeb708a1635d34d1f04d49473926f3396 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 01:58:55 +0200 Subject: [PATCH 17/21] AVRO-4295: [csharp] Reject malformed 10-byte varints in ReadLong The 10th continuation byte is shifted by 63, so any payload bit above bit 0 (b & 0x7E) would be silently dropped, decoding a malformed varint to a wrong value. A valid 64-bit encoding has those bits clear; reject the input with "Invalid long encoding" when they are set. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/IO/BinaryDecoder.cs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 3d56ab9219b..b876f2ce45e 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -84,6 +84,14 @@ public long ReadLong() } b = read(); + // The 10th byte (shift == 63) contributes only bit 63; any higher + // payload bit (b & 0x7E) would be silently dropped by << 63, so a + // valid encoding must have them clear. Reject otherwise. + if (shift == 63 && (b & 0x7E) != 0) + { + throw new AvroException("Invalid long encoding"); + } + n |= (b & 0x7FUL) << shift; shift += 7; } From 98f8da0eb100d8be9aa6550f53f7e99fb4908765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 02:19:33 +0200 Subject: [PATCH 18/21] AVRO-4295: [csharp] Compute prealloc in long; clarify structural-cap message - prealloc was computed with int arithmetic (i + Math.Min(n, cap)), which could overflow when i is near the structural cap. Compute in long, clamp to MaxCollectionStructural, then cast to int. - The structural-cap exception said the "block count" exceeded the limit, but the guard fires when total + count would; reword to "{total} + {count} exceeds ...". Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/main/Generic/GenericReader.cs | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index fb1fac08283..13b3e90fdd4 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -419,8 +419,11 @@ protected virtual object ReadArray(object reuse, ArraySchema writerSchema, Schem // bound the count, so resizing straight to i+n could allocate a // huge array before any element is read; a truncated stream instead // fails within Read() after a bounded growth. Blocks no larger than - // the cap keep the original single-resize fast path. - int prealloc = i + Math.Min(n, MaxCollectionPrealloc); + // the cap keep the original single-resize fast path. Compute in + // long and clamp so a large i near the structural cap cannot + // overflow the int sum. + long preallocLong = Math.Min((long)i + Math.Min(n, MaxCollectionPrealloc), MaxCollectionStructural); + int prealloc = (int)preallocLong; if (GetArraySize(result) < prealloc) ResizeArray(ref result, prealloc); for (int j = 0; j < n; j++, i++) { @@ -676,7 +679,7 @@ private static long EnsureCollectionAvailable(Decoder d, long total, long count, if (count > MaxCollectionStructural - total) { throw new AvroException( - $"Collection block count {count} exceeds the maximum allowed size of {MaxCollectionStructural}"); + $"Collection size {total} + {count} exceeds the maximum allowed size of {MaxCollectionStructural}"); } total += count; From b5d6c6935faec3e2ef982a2bab3db23d73848df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 03:18:39 +0200 Subject: [PATCH 19/21] AVRO-4295: [csharp] Isolate collection tests from AVRO_MAX_COLLECTION_ITEMS The collection-limit tests assume the default caps, but AVRO_MAX_COLLECTION_ITEMS overrides them, so a value set in a shell/CI could make them flaky (fail, or reject a legitimate array). Add [SetUp]/[TearDown] that clear the env var for each test and restore it afterward, so they run deterministically against the defaults. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/test/IO/BinaryCodecTests.cs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index a66aa80e761..d2dc820f37d 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -39,6 +39,24 @@ namespace Avro.Test [TestFixture] public class BinaryCodecTests { + private string _previousMaxCollectionItems; + + // The collection-limit tests assume the default caps. AVRO_MAX_COLLECTION_ITEMS + // overrides them, so a value set in the shell/CI could make these tests + // flaky (or reject a legitimate array). Clear it for each test and restore + // it afterward so the tests are deterministic against the defaults. + [SetUp] + public void ClearCollectionItemsEnv() + { + _previousMaxCollectionItems = Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS"); + Environment.SetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS", null); + } + + [TearDown] + public void RestoreCollectionItemsEnv() + { + Environment.SetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS", _previousMaxCollectionItems); + } /// /// Writes an avro type T with value t into a stream using the encode method e From 22b8caca4d0668e4d007655cf832b31e775d433e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 03:37:13 +0200 Subject: [PATCH 20/21] AVRO-4295: [csharp] Revert ineffective env [SetUp]; document static caps GenericReader captures AVRO_MAX_COLLECTION_ITEMS into static readonly fields at class load, so clearing the env var at runtime in [SetUp] had no effect on the already-computed limits. Remove the misleading [SetUp]/[TearDown] and document that the collection-limit tests assume the process was not started with a custom AVRO_MAX_COLLECTION_ITEMS. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../src/apache/test/IO/BinaryCodecTests.cs | 24 +++++-------------- 1 file changed, 6 insertions(+), 18 deletions(-) diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index d2dc820f37d..5095b2047e5 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -39,24 +39,12 @@ namespace Avro.Test [TestFixture] public class BinaryCodecTests { - private string _previousMaxCollectionItems; - - // The collection-limit tests assume the default caps. AVRO_MAX_COLLECTION_ITEMS - // overrides them, so a value set in the shell/CI could make these tests - // flaky (or reject a legitimate array). Clear it for each test and restore - // it afterward so the tests are deterministic against the defaults. - [SetUp] - public void ClearCollectionItemsEnv() - { - _previousMaxCollectionItems = Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS"); - Environment.SetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS", null); - } - - [TearDown] - public void RestoreCollectionItemsEnv() - { - Environment.SetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS", _previousMaxCollectionItems); - } + // NOTE: the collection-limit tests assume the default caps. GenericReader + // captures AVRO_MAX_COLLECTION_ITEMS into static readonly fields at class + // load, so the value is fixed for the process; these tests therefore + // assume the test process was not started with a custom + // AVRO_MAX_COLLECTION_ITEMS (the normal case). Clearing it at runtime + // would have no effect on the already-captured limits. /// /// Writes an avro type T with value t into a stream using the encode method e From 6a9cb20e09668bf8247057d13afc4263744711f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Mon, 13 Jul 2026 06:31:41 +0200 Subject: [PATCH 21/21] AVRO-4295: [csharp] Bound huge-count reject tests to just over the item cap The array decode and skip tests declared 200,000,000 elements. Since a zero-byte element consumes no input, a future regression of the item-cap guard could let those loops run toward that count and hang/OOM the test process. Use 10,000,001 (just over the default 10,000,000 zero-byte item cap) so the rejection path is still exercised but the failure mode stays bounded. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/csharp/src/apache/test/IO/BinaryCodecTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index 5095b2047e5..2b210fc4789 100644 --- a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs +++ b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs @@ -592,7 +592,7 @@ public void TestReadArrayOfNullRejectsHugeCount() { var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}"); var ms = new MemoryStream(); - new BinaryEncoder(ms).WriteLong(200_000_000); // ~4 byte payload + new BinaryEncoder(ms).WriteLong(10_000_001); // just over the zero-byte item cap; bounded if the guard regresses ms.Position = 0; var reader = new GenericReader(schema, schema); Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); @@ -631,7 +631,7 @@ public void TestSkipArrayOfNullRejectsHugeCount() "{\"type\":\"record\",\"name\":\"Foo\",\"fields\":[" + "{\"name\":\"val\",\"type\":\"int\"}]}"); var ms = new MemoryStream(); - new BinaryEncoder(ms).WriteLong(200_000_000); // arr block count; skipped + new BinaryEncoder(ms).WriteLong(10_000_001); // just over the zero-byte item cap; bounded if the guard regresses ms.Position = 0; var r = new GenericReader(writer, reader); Assert.Throws(() => r.Read(null, new BinaryDecoder(ms)));