Copilot commented on code in PR #3845:
URL: https://github.com/apache/avro/pull/3845#discussion_r3564386936
##########
lang/py/avro/io.py:
##########
@@ -795,26 +850,33 @@ def read_array(self, writers_schema:
avro.schema.ArraySchema, readers_schema: av
The actual count in this case
is the absolute value of the count written.
"""
- read_items = []
+ read_items: List[object] = []
block_count = decoder.read_long()
while block_count != 0:
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ check_max_collection_items(len(read_items), block_count,
self.max_collection_items)
for i in range(block_count):
read_items.append(self.read_data(writers_schema.items,
readers_schema.items, decoder))
block_count = decoder.read_long()
return read_items
def skip_array(self, writers_schema: avro.schema.ArraySchema, decoder:
BinaryDecoder) -> None:
block_count = decoder.read_long()
+ items_skipped = 0
while block_count != 0:
if block_count < 0:
block_size = decoder.read_long()
decoder.skip(block_size)
Review Comment:
`block_size` is read from untrusted input and can be negative on malformed
data. Passing a negative value into `decoder.skip(block_size)` will seek
backwards (since `BinaryDecoder.skip` does not validate `n >= 0`), which can
corrupt decode state or cause non-terminating/incorrect behavior. Reject
negative `block_size` before calling `skip`.
##########
lang/py/avro/io.py:
##########
@@ -834,26 +896,37 @@ def read_map(self, writers_schema: avro.schema.MapSchema,
readers_schema: avro.s
"""
read_items = {}
block_count = decoder.read_long()
+ # Track the number of pairs decoded rather than len(read_items):
repeated
+ # keys collapse in the dict and would otherwise let the cumulative
check
+ # be bypassed by a stream that keeps rewriting the same key.
+ items_read = 0
while block_count != 0:
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ check_max_collection_items(items_read, block_count,
self.max_collection_items)
for i in range(block_count):
key = decoder.read_utf8()
read_items[key] = self.read_data(writers_schema.values,
readers_schema.values, decoder)
+ items_read += block_count
block_count = decoder.read_long()
return read_items
def skip_map(self, writers_schema: avro.schema.MapSchema, decoder:
BinaryDecoder) -> None:
block_count = decoder.read_long()
+ items_skipped = 0
while block_count != 0:
if block_count < 0:
block_size = decoder.read_long()
decoder.skip(block_size)
Review Comment:
`block_size` comes from the wire and may be negative on malformed inputs. A
negative value passed to `decoder.skip(block_size)` will seek backwards (since
`BinaryDecoder.skip` doesn't guard against negative `n`), potentially breaking
subsequent decoding. Validate `block_size >= 0` before skipping.
--
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]
