From 16c0b59ffc7aa861266335d30ae05d98b3f7806f Mon Sep 17 00:00:00 2001 From: Pradeep Rao Date: Wed, 8 Jun 2022 23:08:55 +0000 Subject: [PATCH] decompressors: stop decompressing upon excessive compression ratio (#733) Signed-off-by: Dmitry Rozhkov Co-authored-by: Ryan Hamilton Signed-off-by: Matt Klein Signed-off-by: Pradeep Rao Signed-off-by: Ryan Northey --- docs/root/version_history/current.rst | 1 - source/common/runtime/runtime_features.cc | 1 + .../compression/brotli/common/base.cc | 7 ++--- .../compression/brotli/common/base.h | 3 ++- .../compression/brotli/decompressor/BUILD | 1 + .../decompressor/brotli_decompressor_impl.cc | 21 ++++++++++++++- .../extensions/compression/gzip/common/base.h | 1 - .../compression/gzip/decompressor/BUILD | 1 + .../decompressor/zlib_decompressor_impl.cc | 24 +++++++++++++++++ .../decompressor/zstd_decompressor_impl.cc | 24 +++++++++++++++++ .../decompressor/zstd_decompressor_impl.h | 2 ++ .../brotli_decompressor_impl_test.cc | 26 +++++++++++++++++++ .../compression/gzip/compressor_fuzz_test.cc | 6 +++-- .../zlib_decompressor_impl_test.cc | 25 ++++++++++++++++++ .../zstd_decompressor_impl_test.cc | 21 +++++++++++++++ 15 files changed, 155 insertions(+), 9 deletions(-) diff --git a/docs/root/version_history/current.rst b/docs/root/version_history/current.rst index 96f9d5c9bd98..61bdd6043fc2 100644 --- a/docs/root/version_history/current.rst +++ b/docs/root/version_history/current.rst @@ -15,7 +15,6 @@ Bug Fixes --------- *Changes expected to improve the state of the world and are unlikely to have negative effects* - Removed Config or Runtime ------------------------- *Normally occurs at the end of the* :ref:`deprecation period ` diff --git a/source/common/runtime/runtime_features.cc b/source/common/runtime/runtime_features.cc index 95855fc4495e..b5ee64e960cc 100644 --- a/source/common/runtime/runtime_features.cc +++ b/source/common/runtime/runtime_features.cc @@ -40,6 +40,7 @@ RUNTIME_GUARD(envoy_reloadable_features_correctly_validate_alpn); RUNTIME_GUARD(envoy_reloadable_features_deprecate_global_ints); RUNTIME_GUARD(envoy_reloadable_features_disable_tls_inspector_injection); RUNTIME_GUARD(envoy_reloadable_features_do_not_await_headers_on_upstream_timeout_to_emit_stats); +RUNTIME_GUARD(envoy_reloadable_features_enable_compression_bomb_protection); RUNTIME_GUARD(envoy_reloadable_features_enable_grpc_async_client_cache); RUNTIME_GUARD(envoy_reloadable_features_fix_added_trailers); RUNTIME_GUARD(envoy_reloadable_features_handle_stream_reset_during_hcm_encoding); diff --git a/source/extensions/compression/brotli/common/base.cc b/source/extensions/compression/brotli/common/base.cc index fd364124962c..edbd9be90d9f 100644 --- a/source/extensions/compression/brotli/common/base.cc +++ b/source/extensions/compression/brotli/common/base.cc @@ -6,9 +6,10 @@ namespace Compression { namespace Brotli { namespace Common { -BrotliContext::BrotliContext(const uint32_t chunk_size) - : chunk_size_{chunk_size}, chunk_ptr_{std::make_unique(chunk_size)}, next_in_{}, - next_out_{chunk_ptr_.get()}, avail_in_{0}, avail_out_{chunk_size} {} +BrotliContext::BrotliContext(uint32_t chunk_size, uint32_t max_output_size) + : max_output_size_{max_output_size}, chunk_size_{chunk_size}, + chunk_ptr_{std::make_unique(chunk_size)}, next_in_{}, next_out_{chunk_ptr_.get()}, + avail_in_{0}, avail_out_{chunk_size} {} void BrotliContext::updateOutput(Buffer::Instance& output_buffer) { if (avail_out_ == 0) { diff --git a/source/extensions/compression/brotli/common/base.h b/source/extensions/compression/brotli/common/base.h index fed019c9a297..929081c6ef7f 100644 --- a/source/extensions/compression/brotli/common/base.h +++ b/source/extensions/compression/brotli/common/base.h @@ -12,11 +12,12 @@ namespace Common { // Keeps a `Brotli` compression stream's state. struct BrotliContext { - BrotliContext(const uint32_t chunk_size); + BrotliContext(uint32_t chunk_size, uint32_t max_output_size = 0); void updateOutput(Buffer::Instance& output_buffer); void finalizeOutput(Buffer::Instance& output_buffer); + const uint32_t max_output_size_; const uint32_t chunk_size_; std::unique_ptr chunk_ptr_; const uint8_t* next_in_; diff --git a/source/extensions/compression/brotli/decompressor/BUILD b/source/extensions/compression/brotli/decompressor/BUILD index 252eb0e072ac..18155bfaae36 100644 --- a/source/extensions/compression/brotli/decompressor/BUILD +++ b/source/extensions/compression/brotli/decompressor/BUILD @@ -19,6 +19,7 @@ envoy_cc_library( "//envoy/stats:stats_interface", "//envoy/stats:stats_macros", "//source/common/buffer:buffer_lib", + "//source/common/runtime:runtime_features_lib", "//source/extensions/compression/brotli/common:brotli_base_lib", ], ) diff --git a/source/extensions/compression/brotli/decompressor/brotli_decompressor_impl.cc b/source/extensions/compression/brotli/decompressor/brotli_decompressor_impl.cc index adc2dbb9c731..eb1bb144baa5 100644 --- a/source/extensions/compression/brotli/decompressor/brotli_decompressor_impl.cc +++ b/source/extensions/compression/brotli/decompressor/brotli_decompressor_impl.cc @@ -2,12 +2,24 @@ #include +#include "source/common/runtime/runtime_features.h" + namespace Envoy { namespace Extensions { namespace Compression { namespace Brotli { namespace Decompressor { +namespace { + +// How many times the output buffer is allowed to be bigger than the input +// buffer. This value is used to detect compression bombs. +// TODO(rojkov): Re-design the Decompressor interface to handle compression +// bombs gracefully instead of this quick solution. +constexpr uint32_t MaxInflateRatio = 100; + +} // namespace + BrotliDecompressorImpl::BrotliDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix, const uint32_t chunk_size, const bool disable_ring_buffer_reallocation) @@ -22,7 +34,7 @@ BrotliDecompressorImpl::BrotliDecompressorImpl(Stats::Scope& scope, const std::s void BrotliDecompressorImpl::decompress(const Buffer::Instance& input_buffer, Buffer::Instance& output_buffer) { - Common::BrotliContext ctx(chunk_size_); + Common::BrotliContext ctx(chunk_size_, MaxInflateRatio * input_buffer.length()); for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) { ctx.avail_in_ = input_slice.len_; @@ -58,6 +70,13 @@ bool BrotliDecompressorImpl::process(Common::BrotliContext& ctx, Buffer::Instanc return false; } + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_compression_bomb_protection") && + (output_buffer.length() > ctx.max_output_size_)) { + stats_.brotli_error_.inc(); + return false; + } + ctx.updateOutput(output_buffer); return true; diff --git a/source/extensions/compression/gzip/common/base.h b/source/extensions/compression/gzip/common/base.h index f8b89cb25335..4f427fb90985 100644 --- a/source/extensions/compression/gzip/common/base.h +++ b/source/extensions/compression/gzip/common/base.h @@ -12,7 +12,6 @@ namespace Zlib { /** * Shared code between the compressor and the decompressor. */ -// TODO(junr03): move to extensions tree once the compressor side is moved to extensions. class Base { public: Base(uint64_t chunk_size, std::function zstream_deleter); diff --git a/source/extensions/compression/gzip/decompressor/BUILD b/source/extensions/compression/gzip/decompressor/BUILD index 13939fdcbf2c..2090f49bcd68 100644 --- a/source/extensions/compression/gzip/decompressor/BUILD +++ b/source/extensions/compression/gzip/decompressor/BUILD @@ -21,6 +21,7 @@ envoy_cc_library( "//source/common/buffer:buffer_lib", "//source/common/common:assert_lib", "//source/common/common:minimal_logger_lib", + "//source/common/runtime:runtime_features_lib", "//source/extensions/compression/gzip/common:zlib_base_lib", ], ) diff --git a/source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc b/source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc index 966730c23880..0932638315d1 100644 --- a/source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc +++ b/source/extensions/compression/gzip/decompressor/zlib_decompressor_impl.cc @@ -7,6 +7,7 @@ #include "envoy/common/exception.h" #include "source/common/common/assert.h" +#include "source/common/runtime/runtime_features.h" #include "absl/container/fixed_array.h" @@ -16,6 +17,16 @@ namespace Compression { namespace Gzip { namespace Decompressor { +namespace { + +// How many times the output buffer is allowed to be bigger than the size of +// accumulated input. This value is used to detect compression bombs. +// TODO(rojkov): Re-design the Decompressor interface to handle compression +// bombs gracefully instead of this quick solution. +constexpr uint64_t MaxInflateRatio = 100; + +} // namespace + ZlibDecompressorImpl::ZlibDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix) : ZlibDecompressorImpl(scope, stats_prefix, 4096) {} @@ -43,6 +54,8 @@ void ZlibDecompressorImpl::init(int64_t window_bits) { void ZlibDecompressorImpl::decompress(const Buffer::Instance& input_buffer, Buffer::Instance& output_buffer) { + uint64_t limit = MaxInflateRatio * input_buffer.length(); + for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) { zstream_ptr_->avail_in = input_slice.len_; zstream_ptr_->next_in = static_cast(input_slice.mem_); @@ -50,6 +63,17 @@ void ZlibDecompressorImpl::decompress(const Buffer::Instance& input_buffer, if (zstream_ptr_->avail_out == 0) { updateOutput(output_buffer); } + + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_compression_bomb_protection") && + (output_buffer.length() > limit)) { + stats_.zlib_data_error_.inc(); + ENVOY_LOG(trace, + "excessive decompression ratio detected: output " + "size {} for input size {}", + output_buffer.length(), input_buffer.length()); + return; + } } } diff --git a/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.cc b/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.cc index eb81e4a8587c..7a165da1973c 100644 --- a/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.cc +++ b/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.cc @@ -1,11 +1,23 @@ #include "source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h" +#include "source/common/runtime/runtime_features.h" + namespace Envoy { namespace Extensions { namespace Compression { namespace Zstd { namespace Decompressor { +namespace { + +// How many times the output buffer is allowed to be bigger than the size of +// accumulated input. This value is used to detect compression bombs. +// TODO(rojkov): Re-design the Decompressor interface to handle compression +// bombs gracefully instead of this quick solution. +constexpr uint64_t MaxInflateRatio = 100; + +} // namespace + ZstdDecompressorImpl::ZstdDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix, const ZstdDDictManagerPtr& ddict_manager, uint32_t chunk_size) @@ -14,6 +26,8 @@ ZstdDecompressorImpl::ZstdDecompressorImpl(Stats::Scope& scope, const std::strin void ZstdDecompressorImpl::decompress(const Buffer::Instance& input_buffer, Buffer::Instance& output_buffer) { + uint64_t limit = MaxInflateRatio * input_buffer.length(); + for (const Buffer::RawSlice& input_slice : input_buffer.getRawSlices()) { if (input_slice.len_ > 0) { if (ddict_manager_ && !is_dictionary_set_) { @@ -38,6 +52,16 @@ void ZstdDecompressorImpl::decompress(const Buffer::Instance& input_buffer, if (!process(output_buffer)) { return; } + if (Runtime::runtimeFeatureEnabled( + "envoy.reloadable_features.enable_compression_bomb_protection") && + (output_buffer.length() > limit)) { + stats_.zstd_generic_error_.inc(); + ENVOY_LOG(trace, + "excessive decompression ratio detected: output " + "size {} for input size {}", + output_buffer.length(), input_buffer.length()); + return; + } } } } diff --git a/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h b/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h index 0fa777bea6d7..597a8c72ad0e 100644 --- a/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h +++ b/source/extensions/compression/zstd/decompressor/zstd_decompressor_impl.h @@ -4,6 +4,7 @@ #include "envoy/stats/scope.h" #include "envoy/stats/stats_macros.h" +#include "source/common/common/logger.h" #include "source/extensions/compression/zstd/common/base.h" #include "source/extensions/compression/zstd/common/dictionary_manager.h" @@ -40,6 +41,7 @@ struct ZstdDecompressorStats { */ class ZstdDecompressorImpl : public Common::Base, public Envoy::Compression::Decompressor::Decompressor, + public Logger::Loggable, NonCopyable { public: ZstdDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix, diff --git a/test/extensions/compression/brotli/decompressor/brotli_decompressor_impl_test.cc b/test/extensions/compression/brotli/decompressor/brotli_decompressor_impl_test.cc index c03884f35f9f..57102b7e838e 100644 --- a/test/extensions/compression/brotli/decompressor/brotli_decompressor_impl_test.cc +++ b/test/extensions/compression/brotli/decompressor/brotli_decompressor_impl_test.cc @@ -25,6 +25,32 @@ class BrotliDecompressorImplTest : public testing::Test { static constexpr uint32_t default_input_size{796}; }; +// Detect excessive compression ratio by compressing a long whitespace string +// into a very small chunk of data and decompressing it again. +TEST_F(BrotliDecompressorImplTest, DetectExcessiveCompressionRatio) { + const absl::string_view ten_whitespaces = " "; + Brotli::Compressor::BrotliCompressorImpl compressor{ + default_quality, + default_window_bits, + default_input_block_bits, + false, + Brotli::Compressor::BrotliCompressorImpl::EncoderMode::Default, + 4096}; + Buffer::OwnedImpl buffer; + + for (int i = 0; i < 1000; i++) { + buffer.add(ten_whitespaces); + } + + compressor.compress(buffer, Envoy::Compression::Compressor::State::Finish); + + Buffer::OwnedImpl output_buffer; + Stats::IsolatedStoreImpl stats_store{}; + BrotliDecompressorImpl decompressor{stats_store, "test.", 16, false}; + decompressor.decompress(buffer, output_buffer); + EXPECT_EQ(1, stats_store.counterFromString("test.brotli_error").value()); +} + // Exercises compression and decompression by compressing some data, decompressing it and then // comparing compressor's input/checksum with decompressor's output/checksum. TEST_F(BrotliDecompressorImplTest, CompressAndDecompress) { diff --git a/test/extensions/compression/gzip/compressor_fuzz_test.cc b/test/extensions/compression/gzip/compressor_fuzz_test.cc index 73c592cb1f81..c745abb7a9a2 100644 --- a/test/extensions/compression/gzip/compressor_fuzz_test.cc +++ b/test/extensions/compression/gzip/compressor_fuzz_test.cc @@ -71,8 +71,10 @@ DEFINE_FUZZER(const uint8_t* buf, size_t len) { : Envoy::Compression::Compressor::State::Flush); decompressor.decompress(buffer, full_output); } - RELEASE_ASSERT(full_input.toString() == full_output.toString(), ""); - RELEASE_ASSERT(compressor.checksum() == decompressor.checksum(), ""); + if (stats_store.counterFromString("test.zlib_data_error").value() == 0) { + RELEASE_ASSERT(full_input.toString() == full_output.toString(), ""); + RELEASE_ASSERT(compressor.checksum() == decompressor.checksum(), ""); + } } } // namespace Fuzz diff --git a/test/extensions/compression/gzip/decompressor/zlib_decompressor_impl_test.cc b/test/extensions/compression/gzip/decompressor/zlib_decompressor_impl_test.cc index 0346118a2b22..b895415f8870 100644 --- a/test/extensions/compression/gzip/decompressor/zlib_decompressor_impl_test.cc +++ b/test/extensions/compression/gzip/decompressor/zlib_decompressor_impl_test.cc @@ -122,6 +122,31 @@ TEST_F(ZlibDecompressorImplTest, CallingChecksum) { ASSERT_EQ(0, decompressor.decompression_error_); } +// Detect excessive compression ratio by compressing a long whitespace string +// into a very small chunk of data and decompressing it again. +TEST_F(ZlibDecompressorImplTest, DetectExcessiveCompressionRatio) { + const absl::string_view ten_whitespaces = " "; + Buffer::OwnedImpl buffer; + Extensions::Compression::Gzip::Compressor::ZlibCompressorImpl compressor; + compressor.init( + Extensions::Compression::Gzip::Compressor::ZlibCompressorImpl::CompressionLevel::Standard, + Extensions::Compression::Gzip::Compressor::ZlibCompressorImpl::CompressionStrategy::Standard, + gzip_window_bits, memory_level); + + for (int i = 0; i < 1000; i++) { + buffer.add(ten_whitespaces); + } + + compressor.compress(buffer, Envoy::Compression::Compressor::State::Finish); + + Buffer::OwnedImpl output_buffer; + Stats::IsolatedStoreImpl stats_store{}; + ZlibDecompressorImpl decompressor{stats_store, "test."}; + decompressor.init(gzip_window_bits); + decompressor.decompress(buffer, output_buffer); + ASSERT_EQ(stats_store.counterFromString("test.zlib_data_error").value(), 1); +} + // Exercises compression and decompression by compressing some data, decompressing it and then // comparing compressor's input/checksum with decompressor's output/checksum. TEST_F(ZlibDecompressorImplTest, CompressAndDecompress) { diff --git a/test/extensions/compression/zstd/decompressor/zstd_decompressor_impl_test.cc b/test/extensions/compression/zstd/decompressor/zstd_decompressor_impl_test.cc index 0ab00b1cde9b..be14e14ec11a 100644 --- a/test/extensions/compression/zstd/decompressor/zstd_decompressor_impl_test.cc +++ b/test/extensions/compression/zstd/decompressor/zstd_decompressor_impl_test.cc @@ -149,6 +149,27 @@ TEST_F(ZstdDecompressorImplTest, IllegalConfig) { "assert failure: id != 0. Details: Illegal Zstd dictionary"); } +// Detect excessive compression ratio by compressing a long whitespace string +// into a very small chunk of data and decompressing it again. +TEST_F(ZstdDecompressorImplTest, DetectExcessiveCompressionRatio) { + const absl::string_view ten_whitespaces = " "; + Buffer::OwnedImpl buffer; + for (int i = 0; i < 1000; i++) { + buffer.add(ten_whitespaces); + } + + Zstd::Compressor::ZstdCompressorImpl compressor{default_compression_level_, + default_enable_checksum_, default_strategy_, + default_cdict_manager_, 4096}; + compressor.compress(buffer, Envoy::Compression::Compressor::State::Finish); + + Buffer::OwnedImpl output_buffer; + Stats::IsolatedStoreImpl stats_store{}; + ZstdDecompressorImpl decompressor{stats_store, "test.", default_ddict_manager_, 16}; + decompressor.decompress(buffer, output_buffer); + ASSERT_EQ(stats_store.counterFromString("test.zstd_generic_error").value(), 1); +} + } // namespace // Copy from