Enforce max uncompressed size; use safe LZ4 decompressor, which requires a newer...

Test Plan: folly/io/test, whatever contbuild dreams up for hphp

Reviewed By: meyering@fb.com

Subscribers: meyering, hphp-diffs@, ps, jhj, kma, sainbar, lesha

FB internal diff: D1429372

@override-unit-failures

diff --git a/folly/configure.ac b/folly/configure.ac
index 774cb5b46afdbfe18bf7e8cef93cac1d741013f0..99b4fbbe6380c0ab6346a1833730751e36b79778 100644 (file)
--- a/folly/configure.ac
+++ b/folly/configure.ac
@@ -279,7 +279,7 @@ if test "$ac_cv_func_pthread_yield" = "no"; then
    AC_CHECK_FUNCS([sched_yield])
 fi
 
-AC_CHECK_HEADER([lz4.h], AC_CHECK_LIB([lz4], [main]))
+AC_CHECK_HEADER([lz4.h], AC_CHECK_LIB([lz4], [LZ4_decompress_safe]))
 AC_CHECK_HEADER([snappy.h], AC_CHECK_LIB([snappy], [main]))
 AC_CHECK_HEADER([zlib.h], AC_CHECK_LIB([z], [main]))
 AC_CHECK_HEADER([lzma.h], AC_CHECK_LIB([lzma], [main]))

diff --git a/folly/io/Compression.cpp b/folly/io/Compression.cpp
index 563b878aa6ba67c64ee348f2fd7a186712f2a7f3..f4e12e6297175f1725c5bf32a4a67834e42c4308 100644 (file)
--- a/folly/io/Compression.cpp
+++ b/folly/io/Compression.cpp
@@ -49,7 +49,14 @@ Codec::Codec(CodecType type) : type_(type) { }
 
 // Ensure consistent behavior in the nullptr case
 std::unique_ptr<IOBuf> Codec::compress(const IOBuf* data) {
-  return !data->empty() ? doCompress(data) : IOBuf::create(0);
+  uint64_t len = data->computeChainDataLength();
+  if (len == 0) {
+    return IOBuf::create(0);
+  } else if (len > maxUncompressedLength()) {
+    throw std::runtime_error("Codec: uncompressed length too large");
+  }
+
+  return doCompress(data);
 }
 
 std::unique_ptr<IOBuf> Codec::uncompress(const IOBuf* data,
@@ -86,7 +93,7 @@ bool Codec::doNeedsUncompressedLength() const {
 }
 
 uint64_t Codec::doMaxUncompressedLength() const {
-  return std::numeric_limits<uint64_t>::max() - 1;
+  return UNLIMITED_UNCOMPRESSED_LENGTH;
 }
 
 namespace {
@@ -211,9 +218,7 @@ bool LZ4Codec::doNeedsUncompressedLength() const {
 }
 
 uint64_t LZ4Codec::doMaxUncompressedLength() const {
-  // From lz4.h: "Max supported value is ~1.9GB"; I wish we had something
-  // more accurate.
-  return 1.8 * (uint64_t(1) << 30);
+  return LZ4_MAX_INPUT_SIZE;
 }
 
 std::unique_ptr<IOBuf> LZ4Codec::doCompress(const IOBuf* data) {
@@ -270,15 +275,20 @@ std::unique_ptr<IOBuf> LZ4Codec::doUncompress(
     }
   } else {
     actualUncompressedLength = uncompressedLength;
-    DCHECK_NE(actualUncompressedLength, UNKNOWN_UNCOMPRESSED_LENGTH);
+    if (actualUncompressedLength == UNKNOWN_UNCOMPRESSED_LENGTH ||
+        actualUncompressedLength > maxUncompressedLength()) {
+      throw std::runtime_error("LZ4Codec: invalid uncompressed length");
+    }
   }
 
-  auto out = IOBuf::create(actualUncompressedLength);
   auto p = cursor.peek();
-  int n = LZ4_uncompress(reinterpret_cast<const char*>(p.first),
-                         reinterpret_cast<char*>(out->writableTail()),
-                         actualUncompressedLength);
-  if (n != p.second) {
+  auto out = IOBuf::create(actualUncompressedLength);
+  int n = LZ4_decompress_safe(reinterpret_cast<const char*>(p.first),
+                              reinterpret_cast<char*>(out->writableTail()),
+                              p.second,
+                              actualUncompressedLength);
+
+  if (n != actualUncompressedLength) {
     throw std::runtime_error(to<std::string>(
         "LZ4 decompression returned invalid value ", n));
   }

diff --git a/folly/io/Compression.h b/folly/io/Compression.h
index 569356b13fa291ad86f97537c514def00f6a236b..3b2cbf4ab8b26bca894a9311ed95dc201be971f3 100644 (file)
--- a/folly/io/Compression.h
+++ b/folly/io/Compression.h
@@ -84,6 +84,7 @@ class Codec {
    * Return the maximum length of data that may be compressed with this codec.
    * NO_COMPRESSION and ZLIB support arbitrary lengths;
    * LZ4 supports up to 1.9GiB; SNAPPY supports up to 4GiB.
+   * May return UNLIMITED_UNCOMPRESSED_LENGTH if unlimited.
    */
   uint64_t maxUncompressedLength() const;
 
@@ -120,6 +121,7 @@ class Codec {
    * an empty IOBuf chain will return an empty IOBuf chain.
    */
   static constexpr uint64_t UNKNOWN_UNCOMPRESSED_LENGTH = uint64_t(-1);
+  static constexpr uint64_t UNLIMITED_UNCOMPRESSED_LENGTH = uint64_t(-2);
 
   std::unique_ptr<IOBuf> uncompress(
       const IOBuf* data,