Add missing invalid distance checkandroid-p-preview-1

In the puffin spec, we defined the distance on the wire to be zero
based ([1..32768] on deflate RFC). But I missed it in the implementation and
stored non-zero based distances. However, this is not a bug and since we have
not shipped it yet, it is possible to change.

Another change include, adding proper checks for finding invalid copy length or
distance. Also more unittests for invalid copy length/distance values are
added. Lack of these checks were found by running puffin_fuzzer on cluster fuzz.

Bug: crbug.com/817733
Bug: crbug.com/817686
Test: unittests
Test: test_corpus.py

Change-Id: I38bf630904d7996a3c4f15919960517d26520987

4 files changed, 134 insertions, 58 deletions

diff --git a/src/puff_io_unittest.cc b/src/puff_io_unittest.cc
index 628fd87..64976bd 100644
--- a/src/puff_io_unittest.cc
+++ b/src/puff_io_unittest.cc
@@ -224,35 +224,99 @@ TEST(PuffIOTest, InputOutputTest) {
   ASSERT_EQ(buf.size() - pr.BytesLeft(), epw.Size());
 }
 
-// Testing boundary
-TEST(PuffIOTest, BoundaryTest) {
-  Buffer buf(5);
+// Testing metadata boundary.
+TEST(PuffIOTest, MetadataBoundaryTest) {
   PuffData pd;
   Error error;
-
+  Buffer buf(3);
   BufferPuffWriter pw(buf.data(), buf.size());
-  uint8_t block[] = {10, 11, 12};
+
+  // Block metadata takes two + varied bytes, so on a thre byte buffer, only one
+  // bytes is left for the varied part of metadata.
   pd.type = PuffData::Type::kBlockMetadata;
-  memcpy(pd.block_metadata, block, sizeof(block));
-  pd.length = sizeof(block) + 1;
+  pd.length = 2;
   ASSERT_FALSE(pw.Insert(pd, &error));
   ASSERT_EQ(error, Error::kInsufficientOutput);
+  pd.length = 0;  // length should be at least 1.
+  ASSERT_FALSE(pw.Insert(pd, &error));
+  pd.length = 1;
+  ASSERT_TRUE(pw.Insert(pd, &error));
 
-  BufferPuffWriter pw2(buf.data(), buf.size());
-  pd.length = sizeof(block);
-  ASSERT_TRUE(pw2.Insert(pd, &error));
+  Buffer puff_buffer = {0x00, 0x03, 0x02, 0x00, 0x00};
+  BufferPuffReader pr(puff_buffer.data(), puff_buffer.size());
+  ASSERT_FALSE(pr.GetNext(&pd, &error));
+}
 
-  BufferPuffReader pr(buf.data(), buf.size());
-  ASSERT_TRUE(pr.GetNext(&pd, &error));
-  ASSERT_EQ(pd.type, PuffData::Type::kBlockMetadata);
-  ASSERT_EQ(pd.length, sizeof(block));
-  ASSERT_EQ(pd.block_metadata[0], 10);
+TEST(PuffIOTest, InvalidCopyLengthsDistanceTest) {
+  PuffData pd;
+  Error error;
+  Buffer puff_buffer(20);
+  BufferPuffWriter pw(puff_buffer.data(), puff_buffer.size());
+
+  // Invalid Lenght values.
+  pd.type = PuffData::Type::kLenDist;
+  pd.distance = 1;
+  pd.length = 0;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.length = 1;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.length = 2;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.length = 3;
+  EXPECT_TRUE(pw.Insert(pd, &error));
+  pd.length = 259;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.length = 258;
+  EXPECT_TRUE(pw.Insert(pd, &error));
+
+  // Invalid distance values.
+  pd.length = 3;
+  pd.distance = 0;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.distance = 1;
+  EXPECT_TRUE(pw.Insert(pd, &error));
+  pd.distance = 32769;
+  EXPECT_FALSE(pw.Insert(pd, &error));
+  pd.distance = 32768;
+  EXPECT_TRUE(pw.Insert(pd, &error));
+
+  // First three bytes header, four bytes value lit/len, and four bytes
+  // invalid lit/len.
+  puff_buffer = {0x00, 0x00, 0xFF, 0xFF, 0x80, 0x00,
+                 0x00, 0xFF, 0x82, 0x00, 0x00};
+  BufferPuffReader pr(puff_buffer.data(), puff_buffer.size());
+  EXPECT_TRUE(pr.GetNext(&pd, &error));
+  EXPECT_EQ(pd.type, PuffData::Type::kBlockMetadata);
+  EXPECT_TRUE(pr.GetNext(&pd, &error));
+  EXPECT_EQ(pd.type, PuffData::Type::kLenDist);
+  EXPECT_FALSE(pr.GetNext(&pd, &error));
+}
 
-  BufferPuffReader pr2(buf.data(), sizeof(block));
-  ASSERT_FALSE(pr2.GetNext(&pd, &error));
-  ASSERT_EQ(error, Error::kInsufficientInput);
+TEST(PuffIOTest, InvalidCopyLenghtDistanceBoundaryTest) {
+  PuffData pd;
+  Error error;
+  Buffer puff_buffer(5);
+
+  pd.type = PuffData::Type::kLenDist;
+  pd.distance = 1;
+  pd.length = 129;
+  for (size_t i = 1; i < 2; i++) {
+    BufferPuffWriter pw(puff_buffer.data(), i);
+    EXPECT_FALSE(pw.Insert(pd, &error));
+  }
+
+  pd.length = 130;
+  for (size_t i = 1; i < 3; i++) {
+    BufferPuffWriter pw(puff_buffer.data(), i);
+    EXPECT_FALSE(pw.Insert(pd, &error));
+  }
 
-  // TODO(ahassani): Boundary check for literals and lendist.
+  // First three bytes header, three bytes value lit/len.
+  puff_buffer = {0x00, 0x00, 0xFF, 0xFF, 0x80, 0x00};
+  BufferPuffReader pr(puff_buffer.data(), puff_buffer.size());
+  EXPECT_TRUE(pr.GetNext(&pd, &error));
+  EXPECT_EQ(pd.type, PuffData::Type::kBlockMetadata);
+  EXPECT_FALSE(pr.GetNext(&pd, &error));
 }
 
 TEST(PuffIOTest, LiteralsTest) {
diff --git a/src/puff_reader.cc b/src/puff_reader.cc
index 1365e54..34ef5c3 100644
--- a/src/puff_reader.cc
+++ b/src/puff_reader.cc
@@ -24,10 +24,10 @@ bool BufferPuffReader::GetNext(PuffData* data, Error* error) {
   PuffData& pd = *data;
   size_t length = 0;
   if (state_ == State::kReadingLenDist) {
+    // Boundary check
+    TEST_AND_RETURN_FALSE_SET_ERROR(index_ < puff_size_,
+                                    Error::kInsufficientInput);
     if (puff_buf_in_[index_] & 0x80) {  // Reading length/distance.
-      // Boundary check
-      TEST_AND_RETURN_FALSE_SET_ERROR(index_ < puff_size_,
-                                      Error::kInsufficientInput);
       if ((puff_buf_in_[index_] & 0x7F) < 127) {
         length = puff_buf_in_[index_] & 0x7F;
       } else {
@@ -38,6 +38,8 @@ bool BufferPuffReader::GetNext(PuffData* data, Error* error) {
         length = puff_buf_in_[index_] + 127;
       }
       length += 3;
+      TEST_AND_RETURN_FALSE(length <= 259);
+
       index_++;
 
       // End of block. End of block is similar to length/distance but without
@@ -53,9 +55,13 @@ bool BufferPuffReader::GetNext(PuffData* data, Error* error) {
       TEST_AND_RETURN_FALSE_SET_ERROR(index_ + 1 < puff_size_,
                                       Error::kInsufficientInput);
       auto distance = ReadByteArrayToUint16(&puff_buf_in_[index_]);
+      // The distance in RFC is in the range [1..32768], but in the puff spec,
+      // we write zero-based distance in the puff stream.
+      TEST_AND_RETURN_FALSE_SET_ERROR(distance < (1 << 15),
+                                      Error::kInsufficientInput);
+      distance++;
       index_ += 2;
 
-      TEST_AND_RETURN_FALSE(length < 259);
       pd.type = PuffData::Type::kLenDist;
       pd.length = length;
       pd.distance = distance;
diff --git a/src/puff_writer.cc b/src/puff_writer.cc
index 648bbe5..f39dad9 100644
--- a/src/puff_writer.cc
+++ b/src/puff_writer.cc
@@ -85,7 +85,10 @@ bool BufferPuffWriter::Insert(const PuffData& pd, Error* error) {
     case PuffData::Type::kLenDist:
       DVLOG(2) << "Write length: " << pd.length << " distance: " << pd.distance;
       TEST_AND_RETURN_FALSE(FlushLiterals(error));
-      TEST_AND_RETURN_FALSE_SET_ERROR(pd.length < 259, Error::kInvalidInput);
+      TEST_AND_RETURN_FALSE_SET_ERROR(pd.length <= 258 && pd.length >= 3,
+                                      Error::kInvalidInput);
+      TEST_AND_RETURN_FALSE_SET_ERROR(pd.distance <= 32768 && pd.distance >= 1,
+                                      Error::kInvalidInput);
       if (pd.length < 130) {
         if (puff_buf_out_ != nullptr) {
           // Boundary check
@@ -111,7 +114,8 @@ bool BufferPuffWriter::Insert(const PuffData& pd, Error* error) {
       }
 
       if (puff_buf_out_ != nullptr) {
-        WriteUint16ToByteArray(pd.distance, &puff_buf_out_[index_]);
+        // Write the distance in the range [1..32768] zero-based.
+        WriteUint16ToByteArray(pd.distance - 1, &puff_buf_out_[index_]);
       }
       index_ += 2;
       len_index_ = index_;
@@ -121,6 +125,9 @@ bool BufferPuffWriter::Insert(const PuffData& pd, Error* error) {
     case PuffData::Type::kBlockMetadata:
       DVLOG(2) << "Write block metadata length: " << pd.length;
       TEST_AND_RETURN_FALSE(FlushLiterals(error));
+      TEST_AND_RETURN_FALSE_SET_ERROR(
+          pd.length <= sizeof(pd.block_metadata) && pd.length > 0,
+          Error::kInvalidInput);
       if (puff_buf_out_ != nullptr) {
         // Boundary check
         TEST_AND_RETURN_FALSE_SET_ERROR(index_ + pd.length + 2 <= puff_size_,
@@ -130,7 +137,6 @@ bool BufferPuffWriter::Insert(const PuffData& pd, Error* error) {
       }
       index_ += 2;
 
-      TEST_AND_RETURN_FALSE(pd.length <= sizeof(pd.block_metadata));
       if (puff_buf_out_ != nullptr) {
         memcpy(&puff_buf_out_[index_], pd.block_metadata, pd.length);
       }
diff --git a/src/unittest_common.h b/src/unittest_common.h
index d0bbb02..c03aea4 100644
--- a/src/unittest_common.h
+++ b/src/unittest_common.h
@@ -335,47 +335,47 @@ const Buffer kPuff10 = {
   0x37, 0x20, 0x54, 0x68, 0x65, 0x20, 0x41, 0x6E, 0x64, 0x72, 0x6F, 0x69,
   0x64, 0x20, 0x4F, 0x70, 0x65, 0x6E, 0x20, 0x53, 0x6F, 0x75, 0x72, 0x63,
   0x65, 0x20, 0x50, 0x72, 0x6F, 0x6A, 0x65, 0x63, 0x74, 0x0A, 0x83, 0x00,
-  0x39, 0x0F, 0x4C, 0x69, 0x63, 0x65, 0x6E, 0x73, 0x65, 0x64, 0x20, 0x75,
-  0x6E, 0x64, 0x65, 0x72, 0x20, 0x74, 0x81, 0x00, 0x35, 0x02, 0x70, 0x61,
-  0x63, 0x80, 0x00, 0x07, 0x84, 0x00, 0x1A, 0x0E, 0x2C, 0x20, 0x56, 0x65,
+  0x38, 0x0F, 0x4C, 0x69, 0x63, 0x65, 0x6E, 0x73, 0x65, 0x64, 0x20, 0x75,
+  0x6E, 0x64, 0x65, 0x72, 0x20, 0x74, 0x81, 0x00, 0x34, 0x02, 0x70, 0x61,
+  0x63, 0x80, 0x00, 0x06, 0x84, 0x00, 0x19, 0x0E, 0x2C, 0x20, 0x56, 0x65,
   0x72, 0x73, 0x69, 0x6F, 0x6E, 0x20, 0x32, 0x2E, 0x30, 0x20, 0x28, 0x81,
-  0x00, 0x21, 0x00, 0x22, 0x84, 0x00, 0x1B, 0x02, 0x22, 0x29, 0x3B, 0x81,
-  0x00, 0x43, 0x0E, 0x79, 0x6F, 0x75, 0x20, 0x6D, 0x61, 0x79, 0x20, 0x6E,
-  0x6F, 0x74, 0x20, 0x75, 0x73, 0x65, 0x80, 0x00, 0x44, 0x19, 0x69, 0x73,
+  0x00, 0x20, 0x00, 0x22, 0x84, 0x00, 0x1A, 0x02, 0x22, 0x29, 0x3B, 0x81,
+  0x00, 0x42, 0x0E, 0x79, 0x6F, 0x75, 0x20, 0x6D, 0x61, 0x79, 0x20, 0x6E,
+  0x6F, 0x74, 0x20, 0x75, 0x73, 0x65, 0x80, 0x00, 0x43, 0x19, 0x69, 0x73,
   0x20, 0x66, 0x69, 0x6C, 0x65, 0x20, 0x65, 0x78, 0x63, 0x65, 0x70, 0x74,
   0x20, 0x69, 0x6E, 0x20, 0x63, 0x6F, 0x6D, 0x70, 0x6C, 0x69, 0x61, 0x6E,
-  0x80, 0x00, 0x80, 0x03, 0x77, 0x69, 0x74, 0x68, 0x82, 0x00, 0x68, 0x84,
-  0x00, 0x46, 0x00, 0x2E, 0x81, 0x00, 0x44, 0x00, 0x59, 0x84, 0x00, 0x44,
-  0x03, 0x6F, 0x62, 0x74, 0x61, 0x80, 0x00, 0x2F, 0x00, 0x61, 0x80, 0x00,
-  0x31, 0x00, 0x70, 0x80, 0x00, 0x0E, 0x00, 0x66, 0x89, 0x00, 0x29, 0x01,
-  0x20, 0x61, 0x85, 0x00, 0xB5, 0x82, 0x00, 0x01, 0x0B, 0x68, 0x74, 0x74,
-  0x70, 0x3A, 0x2F, 0x2F, 0x77, 0x77, 0x77, 0x2E, 0x61, 0x82, 0x00, 0xB2,
-  0x05, 0x2E, 0x6F, 0x72, 0x67, 0x2F, 0x6C, 0x83, 0x00, 0x2C, 0x09, 0x73,
-  0x2F, 0x4C, 0x49, 0x43, 0x45, 0x4E, 0x53, 0x45, 0x2D, 0x80, 0x00, 0xB6,
-  0x84, 0x00, 0x36, 0x0C, 0x55, 0x6E, 0x6C, 0x65, 0x73, 0x73, 0x20, 0x72,
-  0x65, 0x71, 0x75, 0x69, 0x72, 0x80, 0x00, 0xF2, 0x04, 0x62, 0x79, 0x20,
-  0x61, 0x70, 0x80, 0x00, 0x96, 0x02, 0x63, 0x61, 0x62, 0x80, 0x00, 0xAC,
+  0x80, 0x00, 0x7F, 0x03, 0x77, 0x69, 0x74, 0x68, 0x82, 0x00, 0x67, 0x84,
+  0x00, 0x45, 0x00, 0x2E, 0x81, 0x00, 0x43, 0x00, 0x59, 0x84, 0x00, 0x43,
+  0x03, 0x6F, 0x62, 0x74, 0x61, 0x80, 0x00, 0x2E, 0x00, 0x61, 0x80, 0x00,
+  0x30, 0x00, 0x70, 0x80, 0x00, 0x0D, 0x00, 0x66, 0x89, 0x00, 0x28, 0x01,
+  0x20, 0x61, 0x85, 0x00, 0xB4, 0x82, 0x00, 0x00, 0x0B, 0x68, 0x74, 0x74,
+  0x70, 0x3A, 0x2F, 0x2F, 0x77, 0x77, 0x77, 0x2E, 0x61, 0x82, 0x00, 0xB1,
+  0x05, 0x2E, 0x6F, 0x72, 0x67, 0x2F, 0x6C, 0x83, 0x00, 0x2B, 0x09, 0x73,
+  0x2F, 0x4C, 0x49, 0x43, 0x45, 0x4E, 0x53, 0x45, 0x2D, 0x80, 0x00, 0xB5,
+  0x84, 0x00, 0x35, 0x0C, 0x55, 0x6E, 0x6C, 0x65, 0x73, 0x73, 0x20, 0x72,
+  0x65, 0x71, 0x75, 0x69, 0x72, 0x80, 0x00, 0xF1, 0x04, 0x62, 0x79, 0x20,
+  0x61, 0x70, 0x80, 0x00, 0x95, 0x02, 0x63, 0x61, 0x62, 0x80, 0x00, 0xAB,
   0x0A, 0x6C, 0x61, 0x77, 0x20, 0x6F, 0x72, 0x20, 0x61, 0x67, 0x72, 0x65,
-  0x80, 0x00, 0x1C, 0x01, 0x74, 0x6F, 0x81, 0x00, 0xB6, 0x10, 0x77, 0x72,
+  0x80, 0x00, 0x1B, 0x01, 0x74, 0x6F, 0x81, 0x00, 0xB5, 0x10, 0x77, 0x72,
   0x69, 0x74, 0x69, 0x6E, 0x67, 0x2C, 0x20, 0x73, 0x6F, 0x66, 0x74, 0x77,
-  0x61, 0x72, 0x65, 0x81, 0x00, 0x47, 0x08, 0x64, 0x69, 0x73, 0x74, 0x72,
-  0x69, 0x62, 0x75, 0x74, 0x8A, 0x01, 0x35, 0x85, 0x00, 0xA4, 0x80, 0x00,
-  0xFB, 0x89, 0x00, 0x21, 0x80, 0x01, 0x37, 0x10, 0x61, 0x6E, 0x20, 0x22,
+  0x61, 0x72, 0x65, 0x81, 0x00, 0x46, 0x08, 0x64, 0x69, 0x73, 0x74, 0x72,
+  0x69, 0x62, 0x75, 0x74, 0x8A, 0x01, 0x34, 0x85, 0x00, 0xA3, 0x80, 0x00,
+  0xFA, 0x89, 0x00, 0x20, 0x80, 0x01, 0x36, 0x10, 0x61, 0x6E, 0x20, 0x22,
   0x41, 0x53, 0x20, 0x49, 0x53, 0x22, 0x20, 0x42, 0x41, 0x53, 0x49, 0x53,
-  0x2C, 0x81, 0x00, 0x45, 0x1E, 0x57, 0x49, 0x54, 0x48, 0x4F, 0x55, 0x54,
+  0x2C, 0x81, 0x00, 0x44, 0x1E, 0x57, 0x49, 0x54, 0x48, 0x4F, 0x55, 0x54,
   0x20, 0x57, 0x41, 0x52, 0x52, 0x41, 0x4E, 0x54, 0x49, 0x45, 0x53, 0x20,
   0x4F, 0x52, 0x20, 0x43, 0x4F, 0x4E, 0x44, 0x49, 0x54, 0x49, 0x4F, 0x4E,
-  0x80, 0x00, 0x0E, 0x0C, 0x46, 0x20, 0x41, 0x4E, 0x59, 0x20, 0x4B, 0x49,
-  0x4E, 0x44, 0x2C, 0x20, 0x65, 0x80, 0x01, 0x33, 0x80, 0x00, 0x68, 0x03,
-  0x65, 0x78, 0x70, 0x72, 0x81, 0x00, 0xC2, 0x80, 0x00, 0xA7, 0x00, 0x69,
-  0x81, 0x01, 0x4F, 0x01, 0x65, 0x64, 0x82, 0x01, 0x3C, 0x02, 0x53, 0x65,
-  0x65, 0x8A, 0x00, 0x83, 0x01, 0x66, 0x6F, 0x83, 0x00, 0x93, 0x07, 0x73,
-  0x70, 0x65, 0x63, 0x69, 0x66, 0x69, 0x63, 0x80, 0x00, 0xDB, 0x0C, 0x6E,
+  0x80, 0x00, 0x0D, 0x0C, 0x46, 0x20, 0x41, 0x4E, 0x59, 0x20, 0x4B, 0x49,
+  0x4E, 0x44, 0x2C, 0x20, 0x65, 0x80, 0x01, 0x32, 0x80, 0x00, 0x67, 0x03,
+  0x65, 0x78, 0x70, 0x72, 0x81, 0x00, 0xC1, 0x80, 0x00, 0xA6, 0x00, 0x69,
+  0x81, 0x01, 0x4E, 0x01, 0x65, 0x64, 0x82, 0x01, 0x3B, 0x02, 0x53, 0x65,
+  0x65, 0x8A, 0x00, 0x82, 0x01, 0x66, 0x6F, 0x83, 0x00, 0x92, 0x07, 0x73,
+  0x70, 0x65, 0x63, 0x69, 0x66, 0x69, 0x63, 0x80, 0x00, 0xDA, 0x0C, 0x6E,
   0x67, 0x75, 0x61, 0x67, 0x65, 0x20, 0x67, 0x6F, 0x76, 0x65, 0x72, 0x6E,
-  0x80, 0x00, 0xD2, 0x06, 0x20, 0x70, 0x65, 0x72, 0x6D, 0x69, 0x73, 0x81,
-  0x01, 0xD7, 0x00, 0x73, 0x80, 0x00, 0xA1, 0x00, 0x64, 0x81, 0x00, 0x47,
-  0x06, 0x6C, 0x69, 0x6D, 0x69, 0x74, 0x61, 0x74, 0x82, 0x00, 0x13, 0x8E,
-  0x00, 0xD8, 0x01, 0x2E, 0x0A, 0xFF, 0x81};
+  0x80, 0x00, 0xD1, 0x06, 0x20, 0x70, 0x65, 0x72, 0x6D, 0x69, 0x73, 0x81,
+  0x01, 0xD6, 0x00, 0x73, 0x80, 0x00, 0xA0, 0x00, 0x64, 0x81, 0x00, 0x46,
+  0x06, 0x6C, 0x69, 0x6D, 0x69, 0x74, 0x61, 0x74, 0x82, 0x00, 0x12, 0x8E,
+  0x00, 0xD7, 0x01, 0x2E, 0x0A, 0xFF, 0x81};
 
 // The following is a sequence of bits starting from the top right and ends in
 // bottom left. It represents the bits in |kDeflate11|. Bits inside the brackets