From 322158b3359d2147aa81d914f31907712ec38973 Mon Sep 17 00:00:00 2001
From: Stella Lau <laus@fb.com>
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<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 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<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));
--
2.34.1