Ismaël Mejía created AVRO-4306:
----------------------------------
Summary: csharp: harden PreresolvingDatumReader
(Specific/Generic<T>) collection allocation
Key: AVRO-4306
URL: https://issues.apache.org/jira/browse/AVRO-4306
Project: Apache Avro
Issue Type: Improvement
Components: csharp
Reporter: Ismaël Mejía
Assignee: Ismaël Mejía
h2. Summary
The collection-allocation hardening added under AVRO-4295 (bounded block counts,
bytes-available check, and bounded preallocation/growth) was applied to the C#
{{DefaultReader}}/{{GenericReader}} path only. The other main datum-reader path,
{{PreresolvingDatumReader<T>}} (the base of {{SpecificDatumReader<T>}} and
{{GenericDatumReader<T>}}), still reads array/map blocks unhardened:
{code}
for (int n = (int)decoder.ReadArrayStart(); n != 0; n =
(int)decoder.ReadArrayNext())
{
arrayAccess.EnsureSize(ref array, i + n); // eager preallocation to i + n
arrayAccess.AddElements(array, n, i, itemReader, decoder, itemReusable);
i += n;
}
{code}
(see {{lang/csharp/src/apache/main/Generic/PreresolvingDatumReader.cs}}
ReadArray
~line 393 and ReadMap ~line 374).
Issues on this path:
* {{(int)ReadArrayStart()/ReadMapStart()}} — no cumulative structural/zero-byte
cap, and the cast can truncate a large count.
* {{EnsureSize(ref array, i + n)}} eagerly preallocates to the declared count,
so
a hostile block count drives a huge up-front allocation (the same DoS the
DefaultReader path now clamps).
* No bytes-remaining check using the element's minimum on-wire size.
Unlike Java (where {{SpecificDatumReader}} extends the hardened
{{GenericDatumReader}} and inherits the checks), C# has two parallel reader
implementations, so the specific/generated-code path is currently unprotected.
h2. Proposed approach
* Factor the limit logic (structural + zero-byte caps, bytes-available check via
{{BinaryDecoder.RemainingBytes()}}, and bounded preallocation/growth) so both
{{DefaultReader}} and {{PreresolvingDatumReader}} share it.
* Thread the writer element/value schema's minimum on-wire size into
{{PreresolvingDatumReader.ReadArray/ReadMap}} and enforce the per-block
checks.
* Clamp {{EnsureSize}} growth like the DefaultReader's preallocation clamp.
* Add tests covering the {{SpecificDatumReader<T>}}/{{GenericDatumReader<T>}}
path
(huge count, zero-byte elements, non-seekable stream).
h2. Context
Raised in review of AVRO-4295 (PR #3860). Kept as a separate task because it
hardens a distinct reader implementation and carries its own regression risk on
the widely used specific-reader path; it should land with dedicated tests rather
than be bolted onto the AVRO-4295 PR late in review.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
