Merge pull request #3866 from bhamiltoncx/fix-undefined-bit-shift

Fix undefined behavior when CheckBitsFit left-shifts 64-bit value by 64 bits

src/idl_parser.cpp

@@ -69,13 +69,16 @@ inline CheckedError NoError() { return CheckedError(false); }
 
 // Ensure that integer values we parse fit inside the declared integer type.
 CheckedError Parser::CheckBitsFit(int64_t val, size_t bits) {
-  // Bits we allow to be used.
-  auto mask = static_cast<int64_t>((1ull << bits) - 1);
-  if (bits < 64 &&
-      (val & ~mask) != 0 &&  // Positive or unsigned.
-      (val |  mask) != -1)   // Negative.
-    return Error("constant does not fit in a " + NumToString(bits) +
-                 "-bit field");
+  // Left-shifting a 64-bit value by 64 bits or more is undefined
+  // behavior (C99 6.5.7), so check *before* we shift.
+  if (bits < 64) {
+    // Bits we allow to be used.
+    auto mask = static_cast<int64_t>((1ull << bits) - 1);
+    if ((val & ~mask) != 0 &&  // Positive or unsigned.
+        (val |  mask) != -1)   // Negative.
+      return Error("constant does not fit in a " + NumToString(bits) +
+                   "-bit field");
+  }
   return NoError();
 }
 

tests/test.cpp

@@ -819,6 +819,33 @@ void EnumStringsTest() {
                         "{ F:[ \"E.C\", \"E.A E.B E.C\" ] }"), true);
 }
 
+void IntegerOutOfRangeTest() {
+  TestError("table T { F:byte; } root_type T; { F:256 }",
+            "constant does not fit");
+  TestError("table T { F:byte; } root_type T; { F:-257 }",
+            "constant does not fit");
+  TestError("table T { F:ubyte; } root_type T; { F:256 }",
+            "constant does not fit");
+  TestError("table T { F:ubyte; } root_type T; { F:-257 }",
+            "constant does not fit");
+  TestError("table T { F:short; } root_type T; { F:65536 }",
+            "constant does not fit");
+  TestError("table T { F:short; } root_type T; { F:-65537 }",
+            "constant does not fit");
+  TestError("table T { F:ushort; } root_type T; { F:65536 }",
+            "constant does not fit");
+  TestError("table T { F:ushort; } root_type T; { F:-65537 }",
+            "constant does not fit");
+  TestError("table T { F:int; } root_type T; { F:4294967296 }",
+            "constant does not fit");
+  TestError("table T { F:int; } root_type T; { F:-4294967297 }",
+            "constant does not fit");
+  TestError("table T { F:uint; } root_type T; { F:4294967296 }",
+            "constant does not fit");
+  TestError("table T { F:uint; } root_type T; { F:-4294967297 }",
+            "constant does not fit");
+}
+
 void UnicodeTest() {
   flatbuffers::Parser parser;
   TEST_EQ(parser.Parse("table T { F:string; }"
@@ -878,6 +905,7 @@ int main(int /*argc*/, const char * /*argv*/[]) {
   ErrorTest();
   ScientificTest();
   EnumStringsTest();
+  IntegerOutOfRangeTest();
   UnicodeTest();
   UnknownFieldsTest();