commit dec17c2e4ad427824314fc283d719fa68026d321
author wub <wub@google.com> Thu Jun 04 13:13:50 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Thu Jun 04 13:14:27 2020 -0700
tree 4986cd4e284462a18cc20b7cf53a2ed83880de23
parent dda9881f1194004004b1cfc625462687babbd66a

Fix gfe_quic_fuzzer.

This change has two fixes. One is a mem leak in the fuzzer. The other one is a buffer overrun in QuicFramer::EncryptPayload, when the packet's length is smaller than the associated data length.

PiperOrigin-RevId: 314789978
Change-Id: Id6aab9572a19d7031403254f39d1677971692447

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index e2139cf..1cba589 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -4443,6 +4443,14 @@
   // Copy in the header, because the encrypter only populates the encrypted
   // plaintext content.
   const size_t ad_len = associated_data.length();
+  if (packet.length() < ad_len) {
+    QUIC_BUG << ENDPOINT
+             << "packet is shorter than associated data length. version:"
+             << version() << ", packet length:" << packet.length()
+             << ", associated data length:" << ad_len;
+    RaiseError(QUIC_ENCRYPTION_FAILURE);
+    return 0;
+  }
   memmove(buffer, associated_data.data(), ad_len);
   // Encrypt the plaintext into the buffer.
   size_t output_length = 0;
@@ -4799,7 +4807,9 @@
           type_byte = IETF_CONNECTION_CLOSE;
           break;
         default:
-          set_detailed_error("Invalid QuicConnectionCloseFrame type.");
+          set_detailed_error(quiche::QuicheStrCat(
+              "Invalid QuicConnectionCloseFrame type: ",
+              static_cast<int>(frame.connection_close_frame->close_type)));
           return RaiseError(QUIC_INTERNAL_ERROR);
       }
       break;

quic/core/quic_framer_test.cc

diff --git a/quic/core/quic_framer_test.cc b/quic/core/quic_framer_test.cc
index fb9c2eb..a6ed3e7 100644
--- a/quic/core/quic_framer_test.cc
+++ b/quic/core/quic_framer_test.cc
@@ -9013,6 +9013,24 @@
   EXPECT_TRUE(CheckEncryption(packet_number, raw.get()));
 }
 
+// Regression test for b/158014497.
+TEST_P(QuicFramerTest, EncryptEmptyPacket) {
+  auto packet = std::make_unique<QuicPacket>(
+      new char[100], 0, true, PACKET_8BYTE_CONNECTION_ID,
+      PACKET_0BYTE_CONNECTION_ID,
+      /*includes_version=*/true,
+      /*includes_diversification_nonce=*/true, PACKET_1BYTE_PACKET_NUMBER,
+      VARIABLE_LENGTH_INTEGER_LENGTH_0,
+      /*retry_token_length=*/0, VARIABLE_LENGTH_INTEGER_LENGTH_0);
+  char buffer[kMaxOutgoingPacketSize];
+  size_t encrypted_length = 1;
+  EXPECT_QUIC_BUG(encrypted_length = framer_.EncryptPayload(
+                      ENCRYPTION_INITIAL, kPacketNumber, *packet, buffer,
+                      kMaxOutgoingPacketSize),
+                  "packet is shorter than associated data length");
+  EXPECT_EQ(0u, encrypted_length);
+}
+
 TEST_P(QuicFramerTest, EncryptPacketWithVersionFlag) {
   QuicFramerPeer::SetPerspective(&framer_, Perspective::IS_CLIENT);
   QuicPacketNumber packet_number = kPacketNumber;