[
https://issues.apache.org/jira/browse/AVRO-4306?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ismaël Mejía reassigned AVRO-4306:
----------------------------------
> 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
> Priority: Major
>
> 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)
