commit 3371b09895d17ce15d1390bb607f735ccb36086c
author fayang <fayang@google.com> Wed Dec 04 07:08:52 2019 -0800
committer Copybara-Service <copybara-worker@google.com> Wed Dec 04 07:09:32 2019 -0800
tree c46900efb91e1e89f4cee3af880697f30de157f6
parent 4cd745db9d9ea6945e776cb16c62f58bbf4883b8

gfe-relnote: Fix an unsigned integer overflow bug in QUIC v99 when processing Ack frame. Not used in prod. Not protected.

PiperOrigin-RevId: 283749707
Change-Id: I0da7578f275146448a29c497c52074e97a257240

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index fdb2074..c5708b9 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -3656,7 +3656,7 @@
     return false;
   }
 
-  if (ack_delay_time_in_us == kVarInt62MaxValue) {
+  if (ack_delay_time_in_us >= (kVarInt62MaxValue >> peer_ack_delay_exponent_)) {
     ack_frame->ack_delay_time = QuicTime::Delta::Infinite();
   } else {
     ack_delay_time_in_us = (ack_delay_time_in_us << peer_ack_delay_exponent_);

quic/core/quic_framer_test.cc

diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index 9dc6fde..ff9653d 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -13867,6 +13867,42 @@
   EXPECT_EQ("", frame.error_details);
 }
 
+// Regression test for crbug/1029636.
+TEST_P(QuicFramerTest, OverlyLargeAckDelay) {
+  if (!VersionHasIetfQuicFrames(framer_.transport_version())) {
+    return;
+  }
+  SetDecrypterLevel(ENCRYPTION_FORWARD_SECURE);
+  // clang-format off
+  unsigned char 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_ACK frame)
+    0x02,
+    // largest acked
+    kVarInt62FourBytes + 0x12, 0x34, 0x56, 0x78,
+    // ack delay time.
+    kVarInt62EightBytes + 0x31, 0x00, 0x00, 0x00, 0xF3, 0xA0, 0x81, 0xE0,
+    // Nr. of additional ack blocks
+    kVarInt62OneByte + 0x00,
+    // first ack block length.
+    kVarInt62FourBytes + 0x12, 0x34, 0x56, 0x77,
+  };
+  // clang-format on
+
+  framer_.ProcessPacket(
+      QuicEncryptedPacket(AsChars(packet99), QUIC_ARRAYSIZE(packet99), false));
+  ASSERT_EQ(1u, visitor_.ack_frames_.size());
+  // Verify ack_delay_time is set correctly.
+  EXPECT_EQ(QuicTime::Delta::Infinite(),
+            visitor_.ack_frames_[0]->ack_delay_time);
+}
+
 }  // namespace
 }  // namespace test
 }  // namespace quic