diff options
author | Amin Hassani <ahassani@google.com> | 2018-03-02 23:58:40 +0000 |
---|---|---|
committer | android-build-merger <android-build-merger@google.com> | 2018-03-02 23:58:40 +0000 |
commit | 8862923799c71450c3ad67495b831588f2a50a59 (patch) | |
tree | 49ce7bafe0791566fb81b49291df98d9e7c1c329 | |
parent | 5b30c5d95f6318e6657cab234905b373743df6d8 (diff) | |
parent | dc5d7972899ec1ab38a3b3a2aa424802b05ea4b1 (diff) | |
download | platform_external_puffin-8862923799c71450c3ad67495b831588f2a50a59.tar.gz platform_external_puffin-8862923799c71450c3ad67495b831588f2a50a59.tar.bz2 platform_external_puffin-8862923799c71450c3ad67495b831588f2a50a59.zip |
Add missing invalid distance check
am: dc5d797289
Change-Id: I66bbf7bdbc17a16945fbc61576806d0ea321757c
-rw-r--r-- | src/puff_io_unittest.cc | 102 | ||||
-rw-r--r-- | src/puff_reader.cc | 14 | ||||
-rw-r--r-- | src/puff_writer.cc | 12 | ||||
-rw-r--r-- | src/unittest_common.h | 64 |
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 |