Fix undefined behavior when decoding varint

Summary: Left shifting `0x7f` by `63` is undefined behavior (i.e. when decoding `0xFFFF...`)

Reviewed By: yfeldblum, terrelln

Differential Revision: D5653353

fbshipit-source-id: c74c9f43a9bc82d15a2223df853dc533cea1478b

diff --git a/folly/Varint.h b/folly/Varint.h
index cd44d2676990c1bfefa4b3d06670f5476103b8bb..2f53dcf13058d0afaf44de6da1ceef0bb1e5204d 100644 (file)
--- a/folly/Varint.h
+++ b/folly/Varint.h
@@ -145,7 +145,7 @@ inline Expected<uint64_t, DecodeVarintError> tryDecodeVarint(Range<T*>& data) {
       b = *p++; val |= (b & 0x7f) << 42; if (b >= 0) break;
       b = *p++; val |= (b & 0x7f) << 49; if (b >= 0) break;
       b = *p++; val |= (b & 0x7f) << 56; if (b >= 0) break;
-      b = *p++; val |= (b & 0x7f) << 63; if (b >= 0) break;
+      b = *p++; val |= (b & 0x01) << 63; if (b >= 0) break;
       return makeUnexpected(DecodeVarintError::TooManyBytes);
     } while (false);
   } else {

diff --git a/folly/test/VarintTest.cpp b/folly/test/VarintTest.cpp
index 6f2b1bd0e40b46623d18286c98c5794213135e65..b8799cd2e5552113d938e7f73003e4105ee79e7b 100644 (file)
--- a/folly/test/VarintTest.cpp
+++ b/folly/test/VarintTest.cpp
@@ -98,6 +98,17 @@ TEST(Varint, Simple) {
              {0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x01});
 }
 
+void testVarintFail(std::initializer_list<uint8_t> bytes) {
+  size_t n = bytes.size();
+  ByteRange data(&*bytes.begin(), n);
+  auto ret = tryDecodeVarint(data);
+  EXPECT_FALSE(ret.hasValue());
+}
+
+TEST(Varint, Fail) {
+  testVarintFail({0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff});
+}
+
 TEST(ZigZag, Simple) {
   EXPECT_EQ(0, encodeZigZag(0));
   EXPECT_EQ(1, encodeZigZag(-1));