Copilot commented on code in PR #3851:
URL: https://github.com/apache/avro/pull/3851#discussion_r3564433306
##########
lang/ruby/lib/avro/data_file.rb:
##########
@@ -28,8 +28,26 @@ module DataFile
META_SCHEMA = Schema.parse('{"type": "map", "values": "bytes"}')
VALID_ENCODINGS = ['binary'].freeze # not used yet
+ # Default upper bound, in bytes, on the size a single data-file block may
+ # decompress to. A block with a very high compression ratio (or a malformed
+ # block) can otherwise expand to far more memory than its compressed size.
+ # Mirrors the Java SDK's decompression limit (AVRO-4247). Overridable with
+ # the AVRO_MAX_DECOMPRESS_LENGTH environment variable.
+ DEFAULT_MAX_DECOMPRESS_LENGTH = 200 * 1024 * 1024 # 200 MiB
+ MAX_DECOMPRESS_LENGTH_ENV = 'AVRO_MAX_DECOMPRESS_LENGTH'
+
class DataFileError < AvroError; end
+ # Raised when a data-file block decompresses to more than the configured
maximum.
+ class DecompressionSizeError < DataFileError; end
+
+ # The maximum number of bytes a single block is allowed to decompress to.
+ def self.max_decompress_length
+ value = ENV[MAX_DECOMPRESS_LENGTH_ENV]
+ return value.to_i if value && value =~ /\A\d+\z/ && value.to_i > 0
+ DEFAULT_MAX_DECOMPRESS_LENGTH
+ end
Review Comment:
`max_decompress_length` calls `to_i` twice on the same ENV value. Parsing
once makes the intent clearer and avoids duplicate conversion work (especially
if a very large numeric string is provided).
##########
lang/ruby/lib/avro/data_file.rb:
##########
@@ -378,7 +430,12 @@ def codec_name; 'zstandard'; end
def decompress(data)
load_zstandard!
- Zstd.decompress(data)
+ uncompressed = Zstd.decompress(data)
+ limit = DataFile.max_decompress_length
+ if uncompressed.bytesize > limit
+ raise DecompressionSizeError, "Decompressed block size exceeds the
maximum allowed of #{limit} bytes"
+ end
Review Comment:
`ZstandardCodec#decompress` fully decompresses the block before applying the
size limit. For a decompression bomb this can still allocate an unbounded
amount of memory (and potentially OOM) before raising `DecompressionSizeError`,
which undermines the intent of enforcing a maximum decompressed block size.
--
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]
