Copilot commented on code in PR #3851:
URL: https://github.com/apache/avro/pull/3851#discussion_r3567488052
##########
lang/ruby/lib/avro/data_file.rb:
##########
@@ -319,10 +340,22 @@ def decompress(compressed)
# (without the RFC1950 header & checksum). See the docs for
# inflateInit2 in https://www.zlib.net/manual.html
zstream = Zlib::Inflate.new(-Zlib::MAX_WBITS)
- data = zstream.inflate(compressed)
- data << zstream.finish
+ limit = DataFile.max_decompress_length
+ data = "".b
+ # Inflate in chunks so an over-large (or malicious) block is rejected
+ # before its full decompressed form is materialized in memory.
+ append = lambda do |chunk|
+ if data.bytesize + chunk.bytesize > limit
+ raise DecompressionSizeError, "Decompressed block size exceeds the
maximum allowed of #{limit} bytes"
+ end
+ data << chunk
+ end
+ zstream.inflate(compressed, &append)
+ remaining = zstream.finish
+ append.call(remaining) unless remaining.empty?
+ data
Review Comment:
`DeflateCodec#decompress` calls `Zlib::Inflate#inflate` with a block
(`zstream.inflate(compressed, &append)`). Ruby only added block/yield support
for `Inflate#inflate` in Ruby 3.1+, but this gem declares
`required_ruby_version >= 2.7`, so this will raise `ArgumentError` on Ruby
2.7–3.0. To keep the size guard while preserving Ruby 2.7 compatibility, fall
back to feeding the inflater the compressed input in small slices (calling
`inflate` repeatedly) when running on older Rubies.
##########
lang/ruby/lib/avro/data_file.rb:
##########
@@ -374,11 +439,30 @@ def load_snappy!
end
class ZstandardCodec
+ # Size of the compressed input fed to the streaming decompressor per
step,
+ # so the decompressed output can be bounded incrementally.
+ ZSTD_DECOMPRESS_CHUNK_SIZE = 16 * 1024
+
def codec_name; 'zstandard'; end
def decompress(data)
load_zstandard!
- Zstd.decompress(data)
+ limit = DataFile.max_decompress_length
+ stream = Zstd::StreamingDecompress.new
+ uncompressed = +''.b
+ offset = 0
+ size = data.bytesize
+ # Decompress the block in chunks so an over-large (or malicious) block
is
+ # rejected before its full decompressed form is materialized in memory.
+ while offset < size
+ out = stream.decompress(data.byteslice(offset,
ZSTD_DECOMPRESS_CHUNK_SIZE))
+ offset += ZSTD_DECOMPRESS_CHUNK_SIZE
+ if uncompressed.bytesize + out.bytesize > limit
+ raise DecompressionSizeError, "Decompressed block size exceeds the
maximum allowed of #{limit} bytes"
+ end
+ uncompressed << out
+ end
+ uncompressed
end
Review Comment:
`ZstandardCodec#decompress` accumulates output from `stream.decompress(...)`
but never calls `stream.finish`. With streaming decompressors, the final bytes
can remain buffered until `finish` is called, so callers may see truncated
output (and the within-limit test may not cover all frames/versions). Consider
appending `stream.finish` (with the same size check) and closing the stream in
an `ensure` to avoid native resource leaks.
--
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]
