Skip to content

Commit

Permalink
decompressors: stop decompressing upon excessive compression ratio (#733
Browse files Browse the repository at this point in the history
)

Signed-off-by: Dmitry Rozhkov <dmitry.rozhkov@intel.com>
Co-authored-by: Ryan Hamilton <rch@google.com>
Signed-off-by: Matt Klein <mklein@lyft.com>
Signed-off-by: Pradeep Rao <pcrao@google.com>
  • Loading branch information
pradeepcrao and RyanTheOptimist committed Jun 8, 2022
1 parent 3b0488f commit cb4ef0b
Show file tree
Hide file tree
Showing 15 changed files with 155 additions and 9 deletions.
1 change: 0 additions & 1 deletion docs/root/version_history/current.rst
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,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 <deprecated>`
Expand Down
1 change: 1 addition & 0 deletions source/common/runtime/runtime_features.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
7 changes: 4 additions & 3 deletions source/extensions/compression/brotli/common/base.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t[]>(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<uint8_t[]>(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) {
Expand Down
3 changes: 2 additions & 1 deletion source/extensions/compression/brotli/common/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<uint8_t[]> chunk_ptr_;
const uint8_t* next_in_;
Expand Down
1 change: 1 addition & 0 deletions source/extensions/compression/brotli/decompressor/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,24 @@

#include <memory>

#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)
Expand All @@ -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_;
Expand Down Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion source/extensions/compression/gzip/common/base.h
Original file line number Diff line number Diff line change
Expand Up @@ -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<void(z_stream*)> zstream_deleter);
Expand Down
1 change: 1 addition & 0 deletions source/extensions/compression/gzip/decompressor/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
],
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand All @@ -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) {}

Expand Down Expand Up @@ -43,13 +54,26 @@ 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<Bytef*>(input_slice.mem_);
while (inflateNext()) {
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;
}
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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)
Expand All @@ -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_) {
Expand All @@ -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;
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -40,6 +41,7 @@ struct ZstdDecompressorStats {
*/
class ZstdDecompressorImpl : public Common::Base,
public Envoy::Compression::Decompressor::Decompressor,
public Logger::Loggable<Logger::Id::decompression>,
NonCopyable {
public:
ZstdDecompressorImpl(Stats::Scope& scope, const std::string& stats_prefix,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 4 additions & 2 deletions test/extensions/compression/gzip/compressor_fuzz_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit cb4ef0b

Please sign in to comment.