commit 0131a5ba98b361e2fb646d36eb071b4f86fb581b
author QUICHE team <quiche-dev@google.com> Wed Mar 20 15:23:27 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Wed Mar 20 16:10:39 2019 -0700
tree 8d900cd29587c021028ce62d25721539f0c4cbc1
parent dc41bf1a71fa71ffd0b7882604a8b3e7b8df70ea

Check new connection ID length before reading it

This CL fixes a buffer overflow in the NEW_CONNECTION_ID read path, and adds sanity checks to prevent similar issues from reoccuring. The issue was found by clusterfuzz:
https://bugs.chromium.org/p/chromium/issues/detail?id=943951#c4

gfe-relnote: trivial security fix when parsing invalid frame, not flag protected
PiperOrigin-RevId: 239486794
Change-Id: I70b8e7b4adfd52afbbcb3308ba7dded0416c884e

quic/core/quic_framer_test.cc

diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 4604a08..ea362eb 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -11201,6 +11201,49 @@
   CheckFramingBoundaries(packet99, QUIC_INVALID_NEW_CONNECTION_ID_DATA);
 }
 
+// Verifies that parsing a NEW_CONNECTION_ID frame with a length above the
+// specified maximum fails.
+TEST_P(QuicFramerTest, InvalidLongNewConnectionIdFrame) {
+  if (framer_.transport_version() != QUIC_VERSION_99) {
+    // The NEW_CONNECTION_ID frame is only for version 99.
+    return;
+  }
+  // clang-format off
+  PacketFragments packet99 = {
+      // type (short header, 4 byte packet number)
+      {"",
+       {0x43}},
+      // connection_id
+      {"",
+       {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10}},
+      // packet number
+      {"",
+       {0x12, 0x34, 0x56, 0x78}},
+      // frame type (IETF_NEW_CONNECTION_ID frame)
+      {"",
+       {0x18}},
+      // error code
+      {"Unable to read new connection ID frame sequence number.",
+       {kVarInt62OneByte + 0x11}},
+      {"Unable to read new connection ID frame connection id length.",
+       {0x13}},  // connection ID length
+      {"Unable to read new connection ID frame connection id.",
+       {0xFE, 0xDC, 0xBA, 0x98, 0x76, 0x54, 0x32, 0x10,
+        0xF0, 0xD2, 0xB4, 0x96, 0x78, 0x5A, 0x3C, 0x1E,
+        0x42, 0x33, 0x42}},
+      {"Can not read new connection ID frame reset token.",
+       {0xb5, 0x69, 0x0f, 0x00, 0x00, 0x00, 0x00, 0x00,
+        0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}}
+  };
+  // clang-format on
+
+  std::unique_ptr<QuicEncryptedPacket> encrypted(
+      AssemblePacketFromFragments(packet99));
+  EXPECT_FALSE(framer_.ProcessPacket(*encrypted));
+  EXPECT_EQ(QUIC_INVALID_NEW_CONNECTION_ID_DATA, framer_.error());
+  EXPECT_EQ("New connection ID length too high.", framer_.detailed_error());
+}
+
 TEST_P(QuicFramerTest, BuildNewConnectionIdFramePacket) {
   if (framer_.transport_version() != QUIC_VERSION_99) {
     // This frame is only for version 99.