Copilot commented on code in PR #3862:
URL: https://github.com/apache/avro/pull/3862#discussion_r3567512151
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -312,12 +381,15 @@ def read_enum(writers_schema, readers_schema, decoder)
def read_array(writers_schema, readers_schema, decoder)
read_items = []
+ min_bytes = min_bytes_per_element(writers_schema.items)
+ total = 0
block_count = decoder.read_long
while block_count != 0
if block_count < 0
block_count = -block_count
_block_size = decoder.read_long
end
+ total = ensure_collection_available(decoder, total, block_count,
min_bytes)
Review Comment:
In `DatumReader#read_array`, the negative-block form reads a block byte-size
but does not validate it. A negative block size is malformed (and `skip_blocks`
already rejects it), so the read path should also raise rather than silently
accepting invalid encodings.
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -121,8 +181,13 @@ def skip_int
def skip_long
b = byte!
+ count = 1
while (b & 0x80) != 0
+ # A 64-bit varint is at most 10 bytes; reject an overlong
continuation
+ # chain so a skipped long can't force scanning unbounded input.
+ raise AvroError, "Varint is too long" if count >= 10
b = byte!
+ count += 1
end
Review Comment:
`BinaryDecoder#skip_long` bounds varints to 10 bytes, but unlike `read_long`
it does not reject 10-byte encodings that set payload bits beyond the 64-bit
range. This means malformed Avro longs that `read_long` would reject can still
be silently accepted when the value is being skipped during projection, which
can lead to inconsistent validation between read vs skip paths.
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -331,12 +403,16 @@ def read_array(writers_schema, readers_schema, decoder)
def read_map(writers_schema, readers_schema, decoder)
read_items = {}
+ # Map keys are strings (>= 1 byte length prefix) plus the value.
+ min_bytes = 1 + min_bytes_per_element(writers_schema.values)
+ total = 0
block_count = decoder.read_long
while block_count != 0
if block_count < 0
block_count = -block_count
_block_size = decoder.read_long
end
+ total = ensure_collection_available(decoder, total, block_count,
min_bytes)
Review Comment:
In `DatumReader#read_map`, the negative-block form reads a block byte-size
but does not validate it. A negative block size is malformed (and `skip_blocks`
already rejects it), so the read path should also raise rather than silently
accepting invalid encodings.
--
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]
