Commit 9883bc37 authored by Nick Terrell's avatar Nick Terrell Committed by Facebook Github Bot

Fix decompression of truncated data

Summary: During decompression, when the data is truncated, `StreamCodec::doUncompress()` loops forever, since it doesn't check forward progress. `Bzip2Codec` does the same.

Reviewed By: chipturner

Differential Revision: D5233052

fbshipit-source-id: 8797a7f06d9afa494eea292a8a5dc980c7571bd0
parent ba74a196
......@@ -341,6 +341,8 @@ std::unique_ptr<IOBuf> StreamCodec::doCompress(IOBuf const* data) {
if (output.empty()) {
buffer->prependChain(addOutputBuffer(output, kDefaultBufferLength));
}
size_t const inputSize = input.size();
size_t const outputSize = output.size();
bool const done = compressStream(input, output, flushOp);
if (done) {
DCHECK(input.empty());
......@@ -348,6 +350,9 @@ std::unique_ptr<IOBuf> StreamCodec::doCompress(IOBuf const* data) {
DCHECK_EQ(current->next(), data);
break;
}
if (inputSize == input.size() && outputSize == output.size()) {
throw std::runtime_error("Codec: No forward progress made");
}
}
buffer->prev()->trimEnd(output.size());
return buffer;
......@@ -395,10 +400,15 @@ std::unique_ptr<IOBuf> StreamCodec::doUncompress(
if (output.empty()) {
buffer->prependChain(addOutputBuffer(output, defaultBufferLength));
}
size_t const inputSize = input.size();
size_t const outputSize = output.size();
bool const done = uncompressStream(input, output, flushOp);
if (done) {
break;
}
if (inputSize == input.size() && outputSize == output.size()) {
throw std::runtime_error("Codec: Truncated data");
}
}
if (!input.empty()) {
throw std::runtime_error("Codec: Junk after end of data");
......@@ -2008,8 +2018,11 @@ std::unique_ptr<IOBuf> Bzip2Codec::doUncompress(
if (stream.avail_out == 0) {
out->prependChain(addOutputBuffer(&stream, kDefaultBufferLength));
}
size_t const outputSize = stream.avail_out;
rc = bzCheck(BZ2_bzDecompress(&stream));
if (outputSize == stream.avail_out) {
throw std::runtime_error("Bzip2Codec: Truncated input");
}
}
out->prev()->trimEnd(stream.avail_out);
......
......@@ -383,15 +383,29 @@ void CompressionCorruptionTest::runSimpleTest(const DataHolder& dh) {
EXPECT_THROW(codec_->uncompress(compressed.get(), uncompressedLength + 1),
std::runtime_error);
auto corrupted = compressed->clone();
corrupted->unshare();
// Truncate the last character
corrupted->prev()->trimEnd(1);
if (!codec_->needsUncompressedLength()) {
EXPECT_THROW(codec_->uncompress(corrupted.get()),
std::runtime_error);
}
EXPECT_THROW(codec_->uncompress(corrupted.get(), uncompressedLength),
std::runtime_error);
corrupted = compressed->clone();
corrupted->unshare();
// Corrupt the first character
++(compressed->writableData()[0]);
++(corrupted->writableData()[0]);
if (!codec_->needsUncompressedLength()) {
EXPECT_THROW(codec_->uncompress(compressed.get()),
EXPECT_THROW(codec_->uncompress(corrupted.get()),
std::runtime_error);
}
EXPECT_THROW(codec_->uncompress(compressed.get(), uncompressedLength),
EXPECT_THROW(codec_->uncompress(corrupted.get(), uncompressedLength),
std::runtime_error);
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment