iemejia commented on code in PR #582:
URL: https://github.com/apache/avro-rs/pull/582#discussion_r3566790949
##########
avro/src/error.rs:
##########
@@ -513,6 +513,12 @@ pub enum Details {
#[error("Failed to decompress with zstd: {0}")]
ZstdDecompress(#[source] std::io::Error),
+ #[error("Failed to decompress with bzip2: {0}")]
+ Bzip2Decompress(#[source] std::io::Error),
+
+ #[error("Failed to decompress with xz: {0}")]
+ XzDecompress(#[source] std::io::Error),
Review Comment:
Fixed in c682b65: `Details::Bzip2Decompress` and `Details::XzDecompress` are
now gated behind the `bzip` and `xz` features (like the Snappy variants), so
they aren't added unconditionally and don't break downstream exhaustive matches
without those features. The codec arms that reference them are already gated by
the same features.
##########
avro/src/codec.rs:
##########
@@ -144,21 +144,34 @@ impl Codec {
/// Decompress a stream of bytes in-place.
pub fn decompress(self, stream: &mut Vec<u8>) -> AvroResult<()> {
+ // Cap the decompressed output at the configured allocation budget so a
+ // small compressed block cannot inflate to an enormous buffer (a
+ // "decompression bomb") and exhaust memory.
+ let max_bytes =
+
crate::util::max_allocation_bytes(crate::util::DEFAULT_MAX_ALLOCATION_BYTES);
*stream = match self {
Codec::Null => return Ok(()),
- Codec::Deflate(_settings) =>
miniz_oxide::inflate::decompress_to_vec(stream).map_err(|e| {
+ Codec::Deflate(_settings) =>
miniz_oxide::inflate::decompress_to_vec_with_limit(stream,
max_bytes).map_err(|e| {
+ use
miniz_oxide::inflate::TINFLStatus::{FailedCannotMakeProgress, BadParam,
Adler32Mismatch, Failed, Done, NeedsMoreInput, HasMoreOutput};
+ // The output would grow past the allocation budget: reject it
as
+ // a decompression bomb rather than allocating without bound.
+ if let HasMoreOutput = e.status {
+ return Error::new(Details::MemoryAllocation {
+ desired: max_bytes.saturating_add(1),
+ maximum: max_bytes,
+ });
+ }
let err = {
- use
miniz_oxide::inflate::TINFLStatus::{FailedCannotMakeProgress, BadParam,
Adler32Mismatch, Failed, Done, NeedsMoreInput, HasMoreOutput};
- use std::io::{Error,ErrorKind};
+ use std::io::{Error as IoError, ErrorKind};
match e.status {
- FailedCannotMakeProgress =>
Error::from(ErrorKind::UnexpectedEof),
- BadParam => Error::other("Unexpected error:
miniz_oxide reported invalid output buffer size. Please report this to avro-rs
developers."), // not possible for _to_vec()
- Adler32Mismatch => Error::from(ErrorKind::InvalidData),
- Failed => Error::from(ErrorKind::InvalidData),
- Done => Error::other("Unexpected error: miniz_oxide
reported an error with a success status. Please report this to avro-rs
developers."),
- NeedsMoreInput =>
Error::from(ErrorKind::UnexpectedEof),
- HasMoreOutput => Error::other("Unexpected error:
miniz_oxide has more data than the output buffer can hold. Please report this
to avro-rs developers."), // not possible for _to_vec()
- other => Error::other(format!("Unexpected error:
{other:?}"))
+ FailedCannotMakeProgress =>
IoError::from(ErrorKind::UnexpectedEof),
+ BadParam => IoError::other("Unexpected error:
miniz_oxide reported invalid output buffer size. Please report this to avro-rs
developers."), // not possible for _to_vec()
+ Adler32Mismatch =>
IoError::from(ErrorKind::InvalidData),
Review Comment:
Fixed in c682b65: updated the deflate `BadParam` message to a generic
'invalid parameter while decompressing' and dropped the stale `// not possible
for _to_vec()` comment, since this path now uses `decompress_to_vec_with_limit`.
--
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]
