From: Nick Terrell <terrelln@fb.com>
Date: Tue, 13 Jun 2017 02:00:54 +0000 (-0700)
Subject: Fix decompression of truncated data
X-Git-Tag: v2017.06.19.00~29
X-Git-Url: http://plrg.eecs.uci.edu/git/?p=folly.git;a=commitdiff_plain;h=9883bc3793fc71b8d4c8d1adf6c6fe796e9131e5;hp=ba74a196a02f5b75d90225bb3ab7cb441ae58de5

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
---

diff --git a/folly/io/Compression.cpp b/folly/io/Compression.cpp
index 8dd19bc7..b1671a56 100644
--- a/folly/io/Compression.cpp
+++ b/folly/io/Compression.cpp
@@ -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);
diff --git a/folly/io/test/CompressionTest.cpp b/folly/io/test/CompressionTest.cpp
index 4c45648a..e15a18da 100644
--- a/folly/io/test/CompressionTest.cpp
+++ b/folly/io/test/CompressionTest.cpp
@@ -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);
 }