Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3567512174
##########
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs:
##########
@@ -430,5 +435,330 @@ 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<AvroException>(() => 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<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ var result = (Array)reader.Read(null, new BinaryDecoder(ms));
+ Assert.AreEqual(100000, result.Length);
+ }
Review Comment:
These tests implicitly assume the default collection caps, but the
implementation allows overriding them via the AVRO_MAX_COLLECTION_ITEMS
environment variable. If that env var is set (e.g., in a developer shell or
CI), this test can fail unexpectedly (or, for low values, reject the
100,000-element array). Consider making the test hermetic by saving/restoring
AVRO_MAX_COLLECTION_ITEMS and forcing it unset for the duration of the test.
##########
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs:
##########
@@ -430,5 +435,330 @@ 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<AvroException>(() => 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<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ var result = (Array)reader.Read(null, new BinaryDecoder(ms));
+ 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<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<AvroException>(() => d.ReadBytes());
+ }
+ }
+
+ // 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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]
+ 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<object>(writer, reader);
+ Assert.Throws<AvroException>(() => r.Read(null, new
BinaryDecoder(ms)));
+ }
Review Comment:
Like TestReadArrayOfNullRejectsHugeCount, this uses a very large declared
count and depends on the default MaxCollectionItems behavior. If
AVRO_MAX_COLLECTION_ITEMS is set higher, the skip loop could run for a very
long time; if the guard regresses, it could also trigger very large
allocations. Consider unsetting AVRO_MAX_COLLECTION_ITEMS within the test and
reducing the count to just above the default cap to keep the failure mode
bounded.
##########
lang/csharp/src/apache/test/IO/BinaryCodecTests.cs:
##########
@@ -430,5 +435,330 @@ 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<AvroException>(() => 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<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<object>(schema, schema);
+ var result = (Array)reader.Read(null, new BinaryDecoder(ms));
+ 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<AvroException>(() => 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => 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<AvroException>(() => d.ReadBytes());
+ }
+ }
+
+ // 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<object>(schema, schema);
+ Assert.Throws<AvroException>(() => reader.Read(null, new
BinaryDecoder(ms)));
+ }
Review Comment:
This test relies on the default MaxCollectionItems limit, but
AVRO_MAX_COLLECTION_ITEMS can raise that cap; in that case, the decode might
proceed and attempt an enormous allocation/loop. Also, using a very large count
(200,000,000) makes the test potentially catastrophic if the guard regresses
(OOM / long hang). Recommend (1) forcing AVRO_MAX_COLLECTION_ITEMS unset inside
the test, and (2) using a count just over the default cap (10,000,001) to keep
the failure mode bounded.
--
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]
