diff --git a/lang/csharp/src/apache/main/Generic/CollectionBounds.cs b/lang/csharp/src/apache/main/Generic/CollectionBounds.cs new file mode 100644 index 00000000000..d9566816fa7 --- /dev/null +++ b/lang/csharp/src/apache/main/Generic/CollectionBounds.cs @@ -0,0 +1,259 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +using System; +using Avro.IO; + +namespace Avro.Generic +{ + /// + /// Shared allocation guards for decoding Avro collections (arrays and maps), + /// used by every generic/specific reader. Avro encodes a collection as one or + /// more blocks, each prefixed with an element count; a malicious or truncated + /// input can declare far more elements than the stream could ever hold, driving + /// an unbounded allocation from a tiny payload. These helpers reject such counts + /// before anything is allocated. The same logic backs both reader + /// implementations ( and + /// ) so the caps cannot drift apart. + /// + internal static class CollectionBounds + { + // 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. + internal static readonly long MaxCollectionItems = ReadCollectionLimit(10_000_000L); + + // The largest array the runtime can allocate. Mirrors + // BinaryDecoder.MaxDotNetArrayLength: the readers size .NET arrays from the + // (cumulative) block count, 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. + internal 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 resize to the block + // count could allocate a huge array before the truncated stream is + // detected. + internal const int MaxCollectionPrealloc = 1024; + + private static long ReadCollectionLimit(long defaultValue) + { + string env = Environment.GetEnvironmentVariable("AVRO_MAX_COLLECTION_ITEMS"); + if (!string.IsNullOrEmpty(env) && long.TryParse(env, out long value) && value >= 0) + { + return value; + } + + return defaultValue; + } + + // Per-thread, per-datum cumulative count of zero-byte-encoded collection + // elements (e.g. an array of nulls). Such elements consume no input, so + // the bytes-remaining check cannot bound them, and a per-collection cap is + // not enough either: a record's schema can declare many zero-byte + // collection fields, each block under the limit but jointly unbounded. The + // budget is therefore cumulative across a whole datum. It is thread-static, + // not reader instance state, because a resolved reader may be reused or + // shared among threads (see PreresolvingDatumReader), which such an + // instance field would make unsafe. + [ThreadStatic] private static long zeroByteItemsRead; + + // Nesting depth of the active decode scope on this thread. A delegated + // reader or a skipped writer field decodes within the enclosing datum's + // scope and accumulates into its budget; only the outermost scope resets + // the running total. + [ThreadStatic] private static int scopeDepth; + + /// + /// Opens a decode scope bounding the cumulative zero-byte-element + /// allocation for the current datum. Scopes nest: a nested scope (a + /// delegated reader or a skipped field) accumulates into the enclosing + /// datum's budget, and only the outermost scope resets the running total, + /// so the cap applies across the whole datum rather than per collection. + /// Dispose the returned scope (via using) once the datum is decoded; + /// the budget is thread-static, so the scope must be closed on the same + /// thread, and it is always closed so state cannot leak into later decodes. + /// + internal static Scope EnterScope() + { + if (scopeDepth == 0) + { + zeroByteItemsRead = 0; + } + + scopeDepth++; + return default; + } + + /// + /// The disposable returned by . A stateless struct + /// so using incurs no allocation; closing the outermost scope resets + /// the per-datum budget. + /// + internal readonly struct Scope : IDisposable + { + /// + public void Dispose() + { + if (--scopeDepth == 0) + { + zeroByteItemsRead = 0; + } + } + } + + /// + /// Minimum number of bytes a single value of the given schema can occupy + /// on the wire. Used to reject an array/map block count that could not be + /// backed by the bytes remaining. A type that encodes to zero bytes + /// returns 0 (not only 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. + /// + internal static int MinBytesPerElement(Schema schema, int depth = 0) + { + if (schema == null) + { + return 0; + } + + switch (schema.Tag) + { + case Schema.Type.Null: + return 0; + case Schema.Type.Float: + return 4; + case Schema.Type.Double: + return 8; + case Schema.Type.Fixed: + return ((FixedSchema)schema).Size; + case Schema.Type.Record: + case Schema.Type.Error: + if (depth > 64) + { + // A cyclic or pathologically deep record. Return 1 (not + // 0) so the collection check stays enabled; a valid + // recursive value always encodes to >= 1 byte. The depth + // guard is applied only here, so zero-byte leaf types + // such as null still return 0 regardless of depth. + return 1; + } + + // Accumulate in a long and clamp so a deeply nested schema + // cannot overflow int into a value <= 0, which would disable + // the collection check. + long total = 0; + foreach (Field f in (RecordSchema)schema) + { + total += MinBytesPerElement(f.Schema, depth + 1); + if (total >= int.MaxValue) + { + return int.MaxValue; + } + } + + return (int)total; + default: + // boolean, int, long, bytes, string, enum, union, array, map: + // all encode to at least one byte. + return 1; + } + } + + /// + /// 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. + /// + /// Decoder the collection is being read from. + /// Running element total across the blocks decoded so far for this collection. + /// Element count declared by the current block. + /// Minimum on-wire size of one element (see ). + internal 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. + if (count < 0) + { + throw new AvroException($"Invalid negative collection block count: {count}"); + } + + // 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} + {count} exceeds the maximum allowed size of {MaxCollectionStructural}"); + } + + total += count; + + if (minBytesPerElement <= 0) + { + // Zero-byte elements (e.g. null) consume no input, so the + // bytes-remaining check cannot bound them. Cap the cumulative + // count across the whole datum, not just this collection: a + // record's schema can declare many zero-byte collection fields, + // each block under the limit but jointly unbounded. + zeroByteItemsRead += count; + if (zeroByteItemsRead > MaxCollectionItems) + { + throw new AvroException( + $"Collection of zero-byte elements ({zeroByteItemsRead}) exceeds the maximum allowed size of {MaxCollectionItems}"); + } + } + else 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"); + } + } + + return total; + } + } +} diff --git a/lang/csharp/src/apache/main/Generic/GenericReader.cs b/lang/csharp/src/apache/main/Generic/GenericReader.cs index 0b945b9ff5e..8163c430755 100644 --- a/lang/csharp/src/apache/main/Generic/GenericReader.cs +++ b/lang/csharp/src/apache/main/Generic/GenericReader.cs @@ -137,7 +137,13 @@ public DefaultReader(Schema writerSchema, Schema readerSchema) /// Object read from the decoder. public T Read(T reuse, Decoder decoder) { - return (T)Read(reuse, WriterSchema, ReaderSchema, decoder); + // Open a fresh zero-byte-element budget for this datum. The cap is + // cumulative across every collection decoded in this datum (see + // CollectionBounds.EnsureCollectionAvailable), not per collection. + using (CollectionBounds.EnterScope()) + { + return (T)Read(reuse, WriterSchema, ReaderSchema, decoder); + } } /// @@ -404,11 +410,52 @@ 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()) + long minBytes = CollectionBounds.MinBytesPerElement(writerSchema.ItemSchema); + long total = 0; + for (long nl = d.ReadArrayStart(); nl != 0; nl = d.ReadArrayNext()) { - if (GetArraySize(result) < (i + n)) ResizeArray(ref result, i + n); + // Reject a block whose element count could not be backed by the + // 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 = CollectionBounds.EnsureCollectionAvailable(d, total, nl, minBytes); + int n = (int)nl; + // 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. 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, CollectionBounds.MaxCollectionPrealloc), CollectionBounds.MaxCollectionStructural); + int prealloc = (int)preallocLong; + if (GetArraySize(result) < prealloc) ResizeArray(ref result, prealloc); for (int j = 0; j < n; j++, i++) { + if (GetArraySize(result) <= i) + { + int current = GetArraySize(result); + // 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; + } + + if (grown > CollectionBounds.MaxCollectionStructural) + { + grown = CollectionBounds.MaxCollectionStructural; + } + + ResizeArray(ref result, (int)grown); + } + SetArrayElement(result, i, Read(GetArrayElement(result, i), writerSchema.ItemSchema, rs.ItemSchema, d)); } } @@ -490,8 +537,13 @@ 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. + long minBytes = 1L + CollectionBounds.MinBytesPerElement(writerSchema.ValueSchema); + long total = 0; + for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++) { string k = d.ReadString(); @@ -661,8 +713,11 @@ protected virtual void Skip(Schema writerSchema, Decoder d) case Schema.Type.Array: { Schema s = (writerSchema as ArraySchema).ItemSchema; + long minBytes = CollectionBounds.MinBytesPerElement(s); + long total = 0; for (long n = d.ReadArrayStart(); n != 0; n = d.ReadArrayNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, n, minBytes); for (long i = 0; i < n; i++) Skip(s, d); } } @@ -670,8 +725,11 @@ protected virtual void Skip(Schema writerSchema, Decoder d) case Schema.Type.Map: { Schema s = (writerSchema as MapSchema).ValueSchema; + long minBytes = 1L + CollectionBounds.MinBytesPerElement(s); + long total = 0; for (long n = d.ReadMapStart(); n != 0; n = d.ReadMapNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, n, minBytes); for (long i = 0; i < n; i++) { d.SkipString(); Skip(s, d); } } } diff --git a/lang/csharp/src/apache/main/Generic/PreresolvingDatumReader.cs b/lang/csharp/src/apache/main/Generic/PreresolvingDatumReader.cs index 53270faecdb..8826930f3d8 100644 --- a/lang/csharp/src/apache/main/Generic/PreresolvingDatumReader.cs +++ b/lang/csharp/src/apache/main/Generic/PreresolvingDatumReader.cs @@ -15,6 +15,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ +using System; using System.Collections.Generic; using System.IO; using Avro.IO; @@ -69,7 +70,15 @@ protected PreresolvingDatumReader(Schema writerSchema, Schema readerSchema) /// public T Read(T reuse, Decoder decoder) { - return (T)_reader(reuse, decoder); + // Open a fresh zero-byte-element budget for this datum. The cap is + // cumulative across every collection decoded in this datum (see + // CollectionBounds.EnsureCollectionAvailable), not per collection. The + // budget is thread-static, so this reader stays safe to share among + // threads as documented. + using (CollectionBounds.EnterScope()) + { + return (T)_reader(reuse, decoder); + } } /// @@ -364,15 +373,24 @@ private ReadItem ResolveMap(MapSchema writerSchema, MapSchema readerSchema) var reader = ResolveReader(ws, rs); var mapAccess = GetMapAccess(readerSchema); - return (r,d) => ReadMap(r, d, mapAccess, reader); + // Map keys are strings (>= 1 byte length prefix) plus the value. + long valueMinBytes = 1L + CollectionBounds.MinBytesPerElement(ws); + return (r,d) => ReadMap(r, d, mapAccess, reader, valueMinBytes); } - private object ReadMap(object reuse, Decoder decoder, MapAccess mapAccess, ReadItem valueReader) + private object ReadMap(object reuse, Decoder decoder, MapAccess mapAccess, ReadItem valueReader, long valueMinBytes) { object map = mapAccess.Create(reuse); - for (int n = (int)decoder.ReadMapStart(); n != 0; n = (int)decoder.ReadMapNext()) + long total = 0; + for (long nl = decoder.ReadMapStart(); nl != 0; nl = decoder.ReadMapNext()) { + // Reject a block whose element count could not be backed by the + // 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 = CollectionBounds.EnsureCollectionAvailable(decoder, total, nl, valueMinBytes); + int n = (int)nl; mapAccess.AddElements(map, n, valueReader, decoder, false); } return map; @@ -383,18 +401,59 @@ private ReadItem ResolveArray(ArraySchema writerSchema, ArraySchema readerSchema var itemReader = ResolveReader(writerSchema.ItemSchema, readerSchema.ItemSchema); var arrayAccess = GetArrayAccess(readerSchema); - return (r, d) => ReadArray(r, d, arrayAccess, itemReader, IsReusable(readerSchema.ItemSchema.Tag)); + long itemMinBytes = CollectionBounds.MinBytesPerElement(writerSchema.ItemSchema); + return (r, d) => ReadArray(r, d, arrayAccess, itemReader, IsReusable(readerSchema.ItemSchema.Tag), itemMinBytes); } - private object ReadArray(object reuse, Decoder decoder, ArrayAccess arrayAccess, ReadItem itemReader, bool itemReusable) + private object ReadArray(object reuse, Decoder decoder, ArrayAccess arrayAccess, ReadItem itemReader, bool itemReusable, long itemMinBytes) { object array = arrayAccess.Create(reuse); int i = 0; - for (int n = (int)decoder.ReadArrayStart(); n != 0; n = (int)decoder.ReadArrayNext()) + // Capacity we have requested from arrayAccess.EnsureSize so far. The + // block is read in bounded chunks that grow this geometrically, so a + // huge declared count on a non-seekable stream (where the + // bytes-remaining check cannot bound it) does not preallocate the + // whole block before any element is read; a truncated stream instead + // faults after a bounded growth. + int capacity = 0; + long total = 0; + for (long nl = decoder.ReadArrayStart(); nl != 0; nl = decoder.ReadArrayNext()) { - arrayAccess.EnsureSize(ref array, i + n); - arrayAccess.AddElements(array, n, i, itemReader, decoder, itemReusable); - i += n; + total = CollectionBounds.EnsureCollectionAvailable(decoder, total, nl, itemMinBytes); + int n = (int)nl; + int remaining = n; + while (remaining > 0) + { + int chunk = Math.Min(remaining, CollectionBounds.MaxCollectionPrealloc); + int needed = i + chunk; + if (capacity < needed) + { + // Grow ~1.5x (amortized O(n), so a legitimate large array + // is not resized on every chunk) plus the current chunk, + // then clamp to the structural cap (which is <= the + // runtime's max array length). Adding `chunk` (not the full + // prealloc bound) avoids over-allocating a small array to + // MaxCollectionPrealloc only to shrink it again at the end. + // The validated element count never exceeds the cap. + long grown = (long)capacity + (capacity >> 1) + chunk; + if (grown < needed) + { + grown = needed; + } + + if (grown > CollectionBounds.MaxCollectionStructural) + { + grown = CollectionBounds.MaxCollectionStructural; + } + + capacity = (int)grown; + arrayAccess.EnsureSize(ref array, capacity); + } + + arrayAccess.AddElements(array, chunk, i, itemReader, decoder, itemReusable); + i += chunk; + remaining -= chunk; + } } arrayAccess.Resize(ref array, i); return array; @@ -486,20 +545,26 @@ private DecoderSkip GetSkip(Schema writerSchema) return d => d.SkipFixed(size); case Schema.Type.Array: var itemSkip = GetSkip(((ArraySchema)writerSchema).ItemSchema); + var arrayItemMinBytes = CollectionBounds.MinBytesPerElement(((ArraySchema)writerSchema).ItemSchema); return d => { + long total = 0; for (long n = d.ReadArrayStart(); n != 0; n = d.ReadArrayNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, n, arrayItemMinBytes); for (long i = 0; i < n; i++) itemSkip(d); } }; case Schema.Type.Map: { var valueSkip = GetSkip(((MapSchema)writerSchema).ValueSchema); + var mapValueMinBytes = 1L + CollectionBounds.MinBytesPerElement(((MapSchema)writerSchema).ValueSchema); return d => { + long total = 0; for (long n = d.ReadMapStart(); n != 0; n = d.ReadMapNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, n, mapValueMinBytes); for (long i = 0; i < n; i++) { d.SkipString(); valueSkip(d); } } }; diff --git a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs index 56aaa6e1815..b876f2ce45e 100644 --- a/lang/csharp/src/apache/main/IO/BinaryDecoder.cs +++ b/lang/csharp/src/apache/main/IO/BinaryDecoder.cs @@ -76,7 +76,22 @@ 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(); + // 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; } @@ -264,11 +279,73 @@ public void SkipFixed(int len) // Read p bytes into a new byte buffer private byte[] read(long p) { - byte[] buffer = new byte[p]; + if (p < 0) + { + throw new AvroException($"Can not read a negative number of bytes: {p}"); + } + + if (p > MaxDotNetArrayLength) + { + // A .NET array cannot be larger than this; reject with a + // consistent AvroException rather than letting new byte[p] throw + // an OverflowException/OutOfMemoryException, mirroring the + // maximum-length guard in ReadString() (the message differs). + throw new AvroException($"Length {p} exceeds the maximum supported array length"); + } + + EnsureAvailableBytes(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; } + /// + /// 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() + { + 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() { int n = stream.ReadByte(); @@ -281,6 +358,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/main/IO/BinaryDecoder.netstandard2.0.cs b/lang/csharp/src/apache/main/IO/BinaryDecoder.netstandard2.0.cs index a37d6fa6c84..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,23 +78,31 @@ 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) { throw new AvroException("Can not deserialize a string with negative length!"); } + EnsureAvailableBytes(length); + if (length > MaxDotNetArrayLength) { 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 c4a0dfaaf31..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,22 +66,35 @@ 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) { throw new AvroException("Can not deserialize a string with negative length!"); } - if (length <= MaxFastReadLength) + EnsureAvailableBytes(length); + + 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); @@ -97,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/main/Reflect/ReflectDefaultReader.cs b/lang/csharp/src/apache/main/Reflect/ReflectDefaultReader.cs index 034cb89f88e..2a4a4006652 100644 --- a/lang/csharp/src/apache/main/Reflect/ReflectDefaultReader.cs +++ b/lang/csharp/src/apache/main/Reflect/ReflectDefaultReader.cs @@ -20,6 +20,7 @@ using System.Collections; using System.Collections.Generic; using System.Globalization; +using Avro.Generic; using Avro.IO; using Avro.Specific; using Newtonsoft.Json.Linq; @@ -496,8 +497,16 @@ protected override object ReadArray(object reuse, ArraySchema writerSchema, Sche } int i = 0; - for (int n = (int)dec.ReadArrayStart(); n != 0; n = (int)dec.ReadArrayNext()) + long minBytes = CollectionBounds.MinBytesPerElement(writerSchema.ItemSchema); + long total = 0; + for (long nl = dec.ReadArrayStart(); nl != 0; nl = dec.ReadArrayNext()) { + // Reject a block whose element count could not be backed by the + // bytes remaining (or, for zero-byte elements, that exceeds the + // cumulative item cap) before allocating for it. Checked on the + // raw long, which also avoids the int cast below overflowing. + total = CollectionBounds.EnsureCollectionAvailable(dec, total, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++, i++) { arrayHelper.Add(Read(null, writerSchema.ItemSchema, rs.ItemSchema, dec)); @@ -532,8 +541,12 @@ protected override object ReadMap(object reuse, MapSchema writerSchema, Schema r map = (System.Collections.IDictionary)Activator.CreateInstance(GetTypeFromSchema(rs, false)); } - for (int n = (int)d.ReadMapStart(); n != 0; n = (int)d.ReadMapNext()) + long minBytes = 1L + CollectionBounds.MinBytesPerElement(writerSchema.ValueSchema); + long total = 0; + for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++) { string k = d.ReadString(); 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/main/Specific/SpecificReader.cs b/lang/csharp/src/apache/main/Specific/SpecificReader.cs index 1019fa36ced..d6223f00a2e 100644 --- a/lang/csharp/src/apache/main/Specific/SpecificReader.cs +++ b/lang/csharp/src/apache/main/Specific/SpecificReader.cs @@ -213,8 +213,16 @@ protected override object ReadArray(object reuse, ArraySchema writerSchema, Sche array = ObjectCreator.Instance.New(getTargetType(readerSchema), Schema.Type.Array) as System.Collections.IList; int i = 0; - for (int n = (int)dec.ReadArrayStart(); n != 0; n = (int)dec.ReadArrayNext()) + long minBytes = CollectionBounds.MinBytesPerElement(writerSchema.ItemSchema); + long total = 0; + for (long nl = dec.ReadArrayStart(); nl != 0; nl = dec.ReadArrayNext()) { + // Reject a block whose element count could not be backed by the + // bytes remaining (or, for zero-byte elements, that exceeds the + // cumulative item cap) before allocating for it. Checked on the + // raw long, which also avoids the int cast below overflowing. + total = CollectionBounds.EnsureCollectionAvailable(dec, total, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++, i++) array.Add(Read(null, writerSchema.ItemSchema, rs.ItemSchema, dec)); } @@ -245,8 +253,12 @@ protected override object ReadMap(object reuse, MapSchema writerSchema, Schema r else map = ObjectCreator.Instance.New(getTargetType(readerSchema), Schema.Type.Map) as System.Collections.IDictionary; - for (int n = (int)d.ReadMapStart(); n != 0; n = (int)d.ReadMapNext()) + long minBytes = 1L + CollectionBounds.MinBytesPerElement(writerSchema.ValueSchema); + long total = 0; + for (long nl = d.ReadMapStart(); nl != 0; nl = d.ReadMapNext()) { + total = CollectionBounds.EnsureCollectionAvailable(d, total, nl, minBytes); + int n = (int)nl; for (int j = 0; j < n; j++) { string k = d.ReadString(); diff --git a/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs b/lang/csharp/src/apache/test/IO/BinaryCodecTests.cs index a638b73fea2..4754c1d6967 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 { @@ -38,6 +39,12 @@ namespace Avro.Test [TestFixture] public class BinaryCodecTests { + // NOTE: the collection-limit tests assume the default caps. CollectionBounds + // (shared by every reader) 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 @@ -278,10 +285,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 +315,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 +441,567 @@ 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); + } + + // 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))); + } + + // 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()); + } + } + + // 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(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))); + } + + // The zero-byte item cap is cumulative across a decoded datum, not per + // collection. A container file carries its own schema, so an attacker can + // declare a record with many array fields, each block individually + // under the limit but jointly unbounded. The first field (1 null) plus a + // second field declaring the full cap (10,000,000) exceeds the cap in + // aggregate and must be rejected before allocating the second field. + [Test] + public void TestRecordOfNullArrayFieldsRejectedCumulativelyAcrossDatum() + { + var schema = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[" + + "{\"name\":\"a\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"b\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}]}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(1); // field a: one null + enc.WriteLong(0); // end of a + enc.WriteLong(10_000_000); // field b: cumulative 10,000,001 > cap; rejected before allocating + enc.WriteLong(0); // end of b (not reached) + ms.Position = 0; + var reader = new GenericReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // The complement: a record whose array fields are jointly under the + // cap decodes normally, and the per-datum budget resets between datums so + // reusing the reader does not accumulate across records. + [Test] + public void TestRecordOfNullArrayFieldsWithinCumulativeLimitReads() + { + var schema = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[" + + "{\"name\":\"a\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"b\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}]}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(3); enc.WriteLong(0); // field a: 3 nulls + enc.WriteLong(3); enc.WriteLong(0); // field b: 3 nulls + var reader = new GenericReader(schema, schema); + // Decode twice on the same reader: the per-datum budget must reset. + for (int i = 0; i < 2; i++) + { + ms.Position = 0; + var rec = (GenericRecord)reader.Read(null, new BinaryDecoder(ms)); + Assert.AreEqual(3, ((System.Collections.IList)rec["a"]).Count); + Assert.AreEqual(3, ((System.Collections.IList)rec["b"]).Count); + } + } + + // On a non-seekable stream the declared element count cannot be 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] + 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(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))); + } + + // 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] + 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()); + } + + // 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() + { + 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()); + } + + // 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 + // 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); + } + } + + // C# has a second reader implementation, GenericDatumReader (based on + // PreresolvingDatumReader), which is public and used directly by + // callers. It must enforce the same collection-allocation caps as the + // DefaultReader used above; the following tests exercise it independently. + + // 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 be rejected by the item cap before building + // the array, rather than decoding e.g. 100,000,000 elements from 5 bytes. + [Test] + public void TestDatumReaderReadArrayOfNullRejectsHugeCount() + { + var schema = Avro.Schema.Parse("{\"type\":\"array\",\"items\":\"null\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(100_000_000); // well over the zero-byte item cap + ms.Position = 0; + var reader = new GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // A non-zero-byte element block whose declared count could not be backed + // by the bytes remaining must be rejected before allocating. + [Test] + public void TestDatumReaderReadArrayRejectsCountBeyondStream() + { + 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 GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // The complement: a legitimate array of nulls under the cap still decodes. + [Test] + public void TestDatumReaderReadArrayOfNullNotFalselyRejected() + { + 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 GenericDatumReader(schema, schema); + var result = (object[])reader.Read(null, new BinaryDecoder(ms)); + Assert.AreEqual(100000, result.Length); + } + + [Test] + public void TestDatumReaderReadMapRejectsCountBeyondStream() + { + var schema = Avro.Schema.Parse("{\"type\":\"map\",\"values\":\"long\"}"); + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(1000000); + ms.Position = 0; + var reader = new GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // The zero-byte item cap is cumulative across a decoded datum, not per + // collection: a record declaring many array fields, each block under + // the limit but jointly over it, must be rejected before allocating the + // offending field. + [Test] + public void TestDatumReaderRecordOfNullArrayFieldsRejectedCumulatively() + { + var schema = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[" + + "{\"name\":\"a\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"b\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}]}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(1); // field a: one null + enc.WriteLong(0); // end of a + enc.WriteLong(10_000_000); // field b: cumulative 10,000,001 > cap; rejected before allocating + enc.WriteLong(0); // end of b (not reached) + ms.Position = 0; + var reader = new GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // The complement: jointly-under-the-cap fields decode, and the per-datum + // budget resets between datums so reusing the reader does not accumulate. + [Test] + public void TestDatumReaderRecordOfNullArrayFieldsWithinLimitReads() + { + var schema = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[" + + "{\"name\":\"a\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"b\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}]}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(3); enc.WriteLong(0); // field a: 3 nulls + enc.WriteLong(3); enc.WriteLong(0); // field b: 3 nulls + var reader = new GenericDatumReader(schema, schema); + for (int i = 0; i < 2; i++) + { + ms.Position = 0; + var rec = (GenericRecord)reader.Read(null, new BinaryDecoder(ms)); + Assert.AreEqual(3, ((object[])rec["a"]).Length); + Assert.AreEqual(3, ((object[])rec["b"]).Length); + } + } + + // 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 TestDatumReaderSkipArrayOfNullRejectsHugeCount() + { + 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(100_000_000); // well over the zero-byte item cap + ms.Position = 0; + var r = new GenericDatumReader(writer, reader); + Assert.Throws(() => r.Read(null, new BinaryDecoder(ms))); + } + + // 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 TestDatumReaderReadArrayRejectsCountAboveIntMax() + { + 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 GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ms))); + } + + // A huge non-zero-byte count on a non-seekable stream (where the + // bytes-remaining check cannot bound it) must not preallocate the whole + // block: the backing store grows on demand, so a truncated stream fails + // with a bounded AvroException instead of a multi-gigabyte allocation. + [Test] + public void TestDatumReaderReadArrayHugeCountOnStreamClampsPreallocation() + { + 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 GenericDatumReader(schema, schema); + Assert.Throws(() => reader.Read(null, new BinaryDecoder(ns))); + } + } + + // A resolved PreresolvingDatumReader is documented as safe to share among + // threads. The per-datum zero-byte-element budget must therefore not be + // reader instance state (a shared counter would let concurrent decodes + // reset and increment each other); it is thread-static. Many threads + // decoding within-cap data through one shared reader must all succeed. + [Test] + public void TestDatumReaderSharedAcrossThreadsIsThreadSafe() + { + var schema = Avro.Schema.Parse( + "{\"type\":\"record\",\"name\":\"R\",\"fields\":[" + + "{\"name\":\"a\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}," + + "{\"name\":\"b\",\"type\":{\"type\":\"array\",\"items\":\"null\"}}]}"); + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteLong(3); enc.WriteLong(0); // field a: 3 nulls + enc.WriteLong(3); enc.WriteLong(0); // field b: 3 nulls + byte[] encoded = ms.ToArray(); + + var reader = new GenericDatumReader(schema, schema); + // Parallel.For aggregates any exception thrown in the body (a spurious + // AvroException from a thread-safety bug, or a failed assertion) into + // an AggregateException and rethrows it, failing the test. A shared + // per-instance zero-byte budget would surface here; the thread-static + // budget must not. + System.Threading.Tasks.Parallel.For(0, 32, _ => + { + for (int i = 0; i < 500; i++) + { + var rec = (GenericRecord)reader.Read(null, new BinaryDecoder(new MemoryStream(encoded))); + Assert.AreEqual(3, ((object[])rec["a"]).Length); + Assert.AreEqual(3, ((object[])rec["b"]).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(); + + protected override void Dispose(bool disposing) + { + if (disposing) + { + inner.Dispose(); + } + + base.Dispose(disposing); + } + } } } diff --git a/lang/csharp/src/apache/test/Reflect/TestArray.cs b/lang/csharp/src/apache/test/Reflect/TestArray.cs index ede5af33594..21dd2da9537 100644 --- a/lang/csharp/src/apache/test/Reflect/TestArray.cs +++ b/lang/csharp/src/apache/test/Reflect/TestArray.cs @@ -82,6 +82,20 @@ public void ListTest() } } + // A malicious array block declaring far more elements than the stream + // could hold must be rejected before allocating, matching the Generic and + // Specific readers. Exercises ReflectDefaultReader.ReadArray. + [TestCase] + public void ListRejectsCountBeyondStream() + { + var schema = Schema.Parse(_simpleList); // array + var ms = new MemoryStream(); + new BinaryEncoder(ms).WriteLong(1000000); // 1,000,000 strings, no data + ms.Seek(0, SeekOrigin.Begin); + var reader = new ReflectReader>(schema, schema); + Assert.Throws(() => reader.Read(new BinaryDecoder(ms))); + } + [TestCase] public void ListRecTest() { diff --git a/lang/csharp/src/apache/test/Specific/SpecificTests.cs b/lang/csharp/src/apache/test/Specific/SpecificTests.cs index 1aa3c3a03ae..1fecddf300d 100644 --- a/lang/csharp/src/apache/test/Specific/SpecificTests.cs +++ b/lang/csharp/src/apache/test/Specific/SpecificTests.cs @@ -511,6 +511,31 @@ public void DeserializeToLogicalTypeWithDefault() } + // A malicious array block declaring far more elements than the stream + // could hold must be rejected before allocating, matching the Generic + // reader. Exercises SpecificDefaultReader.ReadArray (via SpecificReader) + // and SpecificDatumReader (via PreresolvingDatumReader). + [TestCase] + public void TestSpecificReaderRejectsArrayCountBeyondStream() + { + var schema = EmbeddedGenericsRecord._SCHEMA; + + byte[] Malicious() + { + var ms = new MemoryStream(); + var enc = new BinaryEncoder(ms); + enc.WriteUnionIndex(0); // OptionalInt: union branch 0 (null) + enc.WriteLong(1000000); // OptionalIntList: 1,000,000 items, no data + return ms.ToArray(); + } + + var r1 = new SpecificReader(schema, schema); + Assert.Throws(() => r1.Read(null, new BinaryDecoder(new MemoryStream(Malicious())))); + + var r2 = new SpecificDatumReader(schema, schema); + Assert.Throws(() => r2.Read(null, new BinaryDecoder(new MemoryStream(Malicious())))); + } + private static S deserialize(Stream ms, Schema ws, Schema rs) where S : class, ISpecificRecord { long initialPos = ms.Position;