Copilot commented on code in PR #3920:
URL: https://github.com/apache/avro/pull/3920#discussion_r3727471087
##########
lang/java/avro/src/main/java/org/apache/avro/reflect/ReflectDatumReader.java:
##########
@@ -143,6 +144,17 @@ protected Object readArray(Object old, Schema expected,
ResolvingDecoder in) thr
if (l <= 0) {
return newArray(old, 0, expected);
}
+ // Match GenericDatumReader.readArray: before eagerly allocating the
backing
+ // array for the declared block count, verify the input could plausibly
hold
+ // that many elements (guarding against a malformed or truncated payload),
+ // and separately cap element types whose minimum encoded size is zero,
which
+ // the bytes-remaining check cannot bound. Without this a small malformed
+ // record mapped to a Java array field (e.g. long[]) could drive a very
large
+ // eager allocation before any element is read.
+ ensureAvailableCollectionBytes(in, l, expectedType);
+ if (isZeroByteSchema(expectedType)) {
+ SystemLimitException.checkMaxCollectionAllocation(0, l);
+ }
Review Comment:
`checkMaxCollectionAllocation` is only applied to the first array block (`l
= in.readArrayStart()`). For zero-byte element schemas (e.g., `null`,
zero-length fixed, zero-byte records), a malicious payload can split a huge
logical array across multiple blocks so that each block passes this first
check, but the cumulative allocation still exceeds the heap-aware
`maxCollectionAllocation` limit. `GenericDatumReader.readArray` applies
`checkMaxCollectionAllocation(base, l)` before *each* block (including after
`arrayNext`). Consider mirroring that here by tracking a `base`/`total` and
re-checking after every `in.arrayNext()` in the collection/object-array read
loops (and/or factoring an `arrayNext` helper like `GenericDatumReader` does).
--
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]
