From 7525a1600d71b9f9f6b4382cd2356032309d1457 Mon Sep 17 00:00:00 2001 From: "Steinar H. Gunderson" Date: Mon, 4 Jan 2016 12:52:15 +0100 Subject: [PATCH] Fix an issue where the ByteSource path (used for parsing std::string) would incorrectly accept some invalid varints that the other path would not, causing potential CHECK-failures if the unit test were run with --write_uncompressed and a corrupted input file. Found by the afl fuzzer. --- snappy-test.h | 2 ++ snappy.cc | 4 +++- snappy_unittest.cc | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 1 deletion(-) diff --git a/snappy-test.h b/snappy-test.h index dbc55b9..5fb09c7 100644 --- a/snappy-test.h +++ b/snappy-test.h @@ -196,6 +196,7 @@ void Test_Snappy_RandomData(); void Test_Snappy_FourByteOffset(); void Test_SnappyCorruption_TruncatedVarint(); void Test_SnappyCorruption_UnterminatedVarint(); +void Test_SnappyCorruption_OverflowingVarint(); void Test_Snappy_ReadPastEndOfBuffer(); void Test_Snappy_FindMatchLength(); void Test_Snappy_FindMatchLengthRandom(); @@ -500,6 +501,7 @@ static inline int RUN_ALL_TESTS() { snappy::Test_Snappy_FourByteOffset(); snappy::Test_SnappyCorruption_TruncatedVarint(); snappy::Test_SnappyCorruption_UnterminatedVarint(); + snappy::Test_SnappyCorruption_OverflowingVarint(); snappy::Test_Snappy_ReadPastEndOfBuffer(); snappy::Test_Snappy_FindMatchLength(); snappy::Test_Snappy_FindMatchLengthRandom(); diff --git a/snappy.cc b/snappy.cc index d81e6b3..cd01214 100644 --- a/snappy.cc +++ b/snappy.cc @@ -545,7 +545,9 @@ class SnappyDecompressor { if (n == 0) return false; const unsigned char c = *(reinterpret_cast(ip)); reader_->Skip(1); - *result |= static_cast(c & 0x7f) << shift; + uint32 val = c & 0x7f; + if (((val << shift) >> shift) != val) return false; + *result |= val << shift; if (c < 128) { break; } diff --git a/snappy_unittest.cc b/snappy_unittest.cc index 3081662..65ac16a 100644 --- a/snappy_unittest.cc +++ b/snappy_unittest.cc @@ -1007,6 +1007,20 @@ TEST(SnappyCorruption, UnterminatedVarint) { &uncompressed)); } +TEST(SnappyCorruption, OverflowingVarint) { + string compressed, uncompressed; + size_t ulength; + compressed.push_back('\xfb'); + compressed.push_back('\xff'); + compressed.push_back('\xff'); + compressed.push_back('\xff'); + compressed.push_back('\x7f'); + CHECK(!CheckUncompressedLength(compressed, &ulength)); + CHECK(!snappy::IsValidCompressedBuffer(compressed.data(), compressed.size())); + CHECK(!snappy::Uncompress(compressed.data(), compressed.size(), + &uncompressed)); +} + TEST(Snappy, ReadPastEndOfBuffer) { // Check that we do not read past end of input