From 322158b3359d2147aa81d914f31907712ec38973 Mon Sep 17 00:00:00 2001 From: Stella Lau Date: Thu, 17 Aug 2017 16:34:56 -0700 Subject: [PATCH] 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 --- folly/Varint.h | 2 +- folly/test/VarintTest.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/folly/Varint.h b/folly/Varint.h index cd44d267..2f53dcf1 100644 --- a/folly/Varint.h +++ b/folly/Varint.h @@ -145,7 +145,7 @@ inline Expected tryDecodeVarint(Range& 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 6f2b1bd0..b8799cd2 100644 --- 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 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)); -- 2.34.1