iemejia commented on code in PR #3862:
URL: https://github.com/apache/avro/pull/3862#discussion_r3566941889
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -496,12 +550,105 @@ def skip_record(writers_schema, decoder)
end
private
- def skip_blocks(decoder, &blk)
+ # Minimum number of bytes a single value of the given schema can occupy
on
+ # the wire. Used to reject an array/map block count that could not be
+ # backed by the bytes remaining. A type that can encode to zero bytes
+ # (null) returns 0, which disables the check for it (so an array of nulls
+ # is not falsely rejected).
+ def min_bytes_per_element(schema, visited = nil)
+ visited ||= {}.compare_by_identity
+ case schema.type_sym
+ when :null then 0
+ when :float then 4
+ when :double then 8
+ when :fixed then schema.size
+ when :record, :error, :request
+ return 0 if visited[schema]
+ visited[schema] = true
+ total = schema.fields.sum { |field|
min_bytes_per_element(field.type, visited) }
+ visited.delete(schema)
+ total
+ else
+ # boolean, int, long, bytes, string, enum, union, array, map: >= 1
byte
+ # (a union encodes at least a 1-byte branch index).
+ 1
+ end
+ end
+
+ # Reject a collection (array or map) block that could drive an unbounded
+ # allocation, before iterating. A block whose declared element count
could
+ # not be backed by the bytes actually remaining is rejected; a block of
+ # zero-byte elements (where the bytes-remaining check does not apply) is
+ # bounded by a cumulative item cap; and every collection is bounded by a
+ # structural cap. Returns the running total across blocks.
+ #
+ # Both limits default to the same values as the other Avro SDKs and can
be
+ # overridden (to a single value capping both) via the
+ # AVRO_MAX_COLLECTION_ITEMS environment variable.
+ DEFAULT_MAX_COLLECTION_ITEMS = 10_000_000 # Zero-byte element cap.
+ DEFAULT_MAX_COLLECTION_STRUCTURAL = 2147483639 # Integer.MAX_VALUE - 8.
+
+ def collection_limits
+ env = Integer(ENV['AVRO_MAX_COLLECTION_ITEMS'], exception: false)
+ if env && env >= 0
+ [env, env]
+ else
+ [DEFAULT_MAX_COLLECTION_ITEMS, DEFAULT_MAX_COLLECTION_STRUCTURAL]
+ end
+ end
+
+ def ensure_collection_available(decoder, total, count,
min_bytes_per_element)
+ # A negative count here is corrupt/malicious data (the read/skip
callers
+ # already normalized a legitimate negative block count to its absolute
+ # value); reject it explicitly.
+ raise CollectionSizeError, "Invalid negative collection block count:
#{count}" if count < 0
+
+ total += count
+ max_items, max_structural = collection_limits
+
+ # A structural cap covers all collections, including decoders that
cannot
+ # report their remaining bytes.
+ if total > max_structural
+ raise CollectionSizeError, "Collection size #{total} exceeds the
maximum allowed size of #{max_structural}"
+ end
+
+ if min_bytes_per_element <= 0
+ # Zero-byte elements (e.g. null) consume no input, so the
+ # bytes-remaining check cannot bound them; cap by item count.
+ if total > max_items
+ raise CollectionSizeError, "Collection of zero-byte elements
(#{total}) exceeds the maximum allowed size of #{max_items}"
+ end
+ elsif decoder.respond_to?(:bytes_remaining)
+ # A decoder that implements the read protocol but not
#bytes_remaining
+ # (e.g. a custom decoder) cannot report the remaining size; skip the
+ # byte check for it rather than raising NoMethodError.
+ remaining = decoder.bytes_remaining
+ # Compare via integer division rather than multiplying, so a huge
count
+ # does not create a large intermediate product.
+ if remaining && count > remaining / min_bytes_per_element
+ raise CollectionSizeError, "Collection claims #{count} elements
with at least #{min_bytes_per_element} bytes each, but only #{remaining} bytes
are available"
+ end
+ end
+
+ total
+ end
+
+ def skip_blocks(decoder, min_bytes = 0, &blk)
+ total = 0
block_count = decoder.read_long
while block_count != 0
if block_count < 0
- decoder.skip(decoder.read_long)
+ # A negative count declares abs(count) items preceded by a block
+ # byte-size. Bound the count too (so it can't bypass the caps), and
+ # reject a negative byte-size (which would seek the reader
backwards)
+ # before skipping the whole block by its size.
+ block_count = -block_count
+ block_size = decoder.read_long
+ raise AvroError, "Invalid negative block size: #{block_size}" if
block_size < 0
+ total = ensure_collection_available(decoder, total, block_count,
min_bytes)
+ decoder.skip(block_size)
Review Comment:
Fixed in ebf1f8be6c: the negative-block skip path now rejects a block_size
larger than the bytes remaining (when the reader can report them) before
decoder.skip, so a truncated payload can't seek past EOF. Added a regression
test.
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -496,12 +550,105 @@ def skip_record(writers_schema, decoder)
end
private
- def skip_blocks(decoder, &blk)
+ # Minimum number of bytes a single value of the given schema can occupy
on
+ # the wire. Used to reject an array/map block count that could not be
+ # backed by the bytes remaining. A type that can encode to zero bytes
+ # (null) returns 0, which disables the check for it (so an array of nulls
+ # is not falsely rejected).
+ def min_bytes_per_element(schema, visited = nil)
+ visited ||= {}.compare_by_identity
+ case schema.type_sym
+ when :null then 0
+ when :float then 4
+ when :double then 8
+ when :fixed then schema.size
+ when :record, :error, :request
+ return 0 if visited[schema]
+ visited[schema] = true
+ total = schema.fields.sum { |field|
min_bytes_per_element(field.type, visited) }
+ visited.delete(schema)
+ total
+ else
+ # boolean, int, long, bytes, string, enum, union, array, map: >= 1
byte
+ # (a union encodes at least a 1-byte branch index).
+ 1
+ end
+ end
+
+ # Reject a collection (array or map) block that could drive an unbounded
+ # allocation, before iterating. A block whose declared element count
could
+ # not be backed by the bytes actually remaining is rejected; a block of
+ # zero-byte elements (where the bytes-remaining check does not apply) is
+ # bounded by a cumulative item cap; and every collection is bounded by a
+ # structural cap. Returns the running total across blocks.
+ #
+ # Both limits default to the same values as the other Avro SDKs and can
be
+ # overridden (to a single value capping both) via the
+ # AVRO_MAX_COLLECTION_ITEMS environment variable.
+ DEFAULT_MAX_COLLECTION_ITEMS = 10_000_000 # Zero-byte element cap.
+ DEFAULT_MAX_COLLECTION_STRUCTURAL = 2147483639 # Integer.MAX_VALUE - 8.
+
+ def collection_limits
+ env = Integer(ENV['AVRO_MAX_COLLECTION_ITEMS'], exception: false)
+ if env && env >= 0
+ [env, env]
+ else
+ [DEFAULT_MAX_COLLECTION_ITEMS, DEFAULT_MAX_COLLECTION_STRUCTURAL]
+ end
+ end
+
+ def ensure_collection_available(decoder, total, count,
min_bytes_per_element)
+ # A negative count here is corrupt/malicious data (the read/skip
callers
+ # already normalized a legitimate negative block count to its absolute
+ # value); reject it explicitly.
+ raise CollectionSizeError, "Invalid negative collection block count:
#{count}" if count < 0
Review Comment:
Fixed in ebf1f8be6c: read_long now bounds a 64-bit long to at most 10 bytes
and raises AvroError on a longer continuation chain, so a malicious varint
can't force unbounded Integer growth before any caller-side check runs. Added a
regression test.
--
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]
