Add missing invalid distance check

am: dc5d797289

Change-Id: I66bbf7bdbc17a16945fbc61576806d0ea321757c

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