commit da9fafd2d94f51a79adcdda07157e7b477df2161
author dschinazi <dschinazi@google.com> Mon Mar 16 15:32:55 2020 -0700
committer Copybara-Service <copybara-worker@google.com> Mon Mar 16 15:33:32 2020 -0700
tree 70cf16995a9151b6ca66494572fa1f77989181e3
parent c30a40adf36f5d3b53b6c451ae1637780b9d940d

Prevent crash when trying to serialize packet with missing encryption key

b/151452116 shows GFE crashing in QuicFramer due to dereferencing a null pointer. This CL adds checks for this and triggers a QUIC_BUG instead of crashing. Our goal is to have the issue reproduce with the QUIC_BUG so we can investigate further what's causing us to get into that state.

gfe-relnote: replace crash with GFE_BUG, not flag-protected
PiperOrigin-RevId: 301250535
Change-Id: I4e88a9de2cec50867fd4c166a3688147670dd264

quic/core/quic_framer.cc

diff --git a/quic/core/quic_framer.cc b/quic/core/quic_framer.cc
index eeaf258..0b296df 100644
--- a/quic/core/quic_framer.cc
+++ b/quic/core/quic_framer.cc
@@ -4256,6 +4256,14 @@
     return false;
   }
 
+  if (encrypter_[level] == nullptr) {
+    QUIC_BUG
+        << ENDPOINT
+        << "Attempted to apply header protection without encrypter at level "
+        << EncryptionLevelToString(level) << " using " << version_;
+    return false;
+  }
+
   std::string mask = encrypter_[level]->GenerateHeaderProtectionMask(sample);
   if (mask.empty()) {
     QUIC_BUG << "Unable to generate header protection mask.";
@@ -4475,6 +4483,12 @@
 
 size_t QuicFramer::GetCiphertextSize(EncryptionLevel level,
                                      size_t plaintext_size) const {
+  if (encrypter_[level] == nullptr) {
+    QUIC_BUG << ENDPOINT
+             << "Attempted to get ciphertext size without encrypter at level "
+             << EncryptionLevelToString(level) << " using " << version_;
+    return plaintext_size;
+  }
   return encrypter_[level]->GetCiphertextSize(plaintext_size);
 }
 

quic/core/quic_packet_creator.cc

diff --git a/quic/core/quic_packet_creator.cc b/quic/core/quic_packet_creator.cc
index 923b219..cea1a1b 100644
--- a/quic/core/quic_packet_creator.cc
+++ b/quic/core/quic_packet_creator.cc
@@ -679,6 +679,15 @@
                 << QuicFramesToString(queued_frames_) << " at encryption_level "
                 << EncryptionLevelToString(packet_.encryption_level);
 
+  if (!framer_->HasEncrypterOfEncryptionLevel(packet_.encryption_level)) {
+    QUIC_BUG << ENDPOINT << "Attempting to serialize " << header
+             << QuicFramesToString(queued_frames_)
+             << " at missing encryption_level "
+             << EncryptionLevelToString(packet_.encryption_level) << " using "
+             << framer_->version();
+    return;
+  }
+
   DCHECK_GE(max_plaintext_size_, packet_size_);
   // Use the packet_size_ instead of the buffer size to ensure smaller
   // packet sizes are properly used.