Signal error in HttpDecoder on empty PUSH_PROMISE frame.

Currently on an empty, invalid PUSH_PROMISE frame HttpDecoder transitions from
STATE_READING_FRAME_LENGTH directly to STATE_FINISH_PARSING, skipping
STATE_READING_FRAME_PAYLOAD, which results in calling
Visitor::OnPushPromiseFrameEnd() without calling
Visitor::OnPushPromiseFrameStart().  This is wrong and can cause QuicSpdyStream
to crash.

This was caught by ClusterFuzz at https://crbug.com/1001823.

Also add tests for other empty frames, and sanity DCHECKs in QuicSpdyStream.

gfe-relnote: n/a, change to QUIC v99-only code.  Protected by existing disabled
gfe2_reloadable_flag_quic_enable_version_99.
PiperOrigin-RevId: 270386637
Change-Id: I0c1944d1df300136d27367679e3128dd45e9bfd3

quic/core/http/http_decoder.cc

diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index c9519fe..e1fa36e 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -153,6 +153,10 @@
       continue_processing = visitor_->OnSettingsFrameStart(header_length);
       break;
     case static_cast<uint64_t>(HttpFrameType::PUSH_PROMISE):
+      if (current_frame_length_ == 0) {
+        RaiseError(QUIC_INVALID_FRAME_DATA, "Corrupt PUSH_PROMISE frame.");
+        return false;
+      }
       break;
     case static_cast<uint64_t>(HttpFrameType::GOAWAY):
       break;

quic/core/http/http_decoder_test.cc

diff --git a/quic/core/http/http_decoder_test.cc b/quic/core/http/http_decoder_test.cc
index 6847ecb..3abc8be 100644
--- a/quic/core/http/http_decoder_test.cc
+++ b/quic/core/http/http_decoder_test.cc
@@ -919,7 +919,78 @@
       EXPECT_EQ(test_data.error_message, decoder.error_detail());
     }
   }
-}  // namespace test
+}
+
+TEST_F(HttpDecoderTest, EmptyCancelPushFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "03"    // type (CANCEL_PUSH)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnError(&decoder_));
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_INVALID_FRAME_DATA, decoder_.error());
+  EXPECT_EQ("Unable to read push_id", decoder_.error_detail());
+}
+
+TEST_F(HttpDecoderTest, EmptySettingsFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "04"    // type (SETTINGS)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnSettingsFrameStart(2));
+
+  SettingsFrame empty_frame;
+  EXPECT_CALL(visitor_, OnSettingsFrame(empty_frame));
+
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_NO_ERROR, decoder_.error());
+  EXPECT_EQ("", decoder_.error_detail());
+}
+
+// Regression test for https://crbug.com/1001823.
+TEST_F(HttpDecoderTest, EmptyPushPromiseFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "05"    // type (PUSH_PROMISE)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnError(&decoder_));
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_INVALID_FRAME_DATA, decoder_.error());
+  EXPECT_EQ("Corrupt PUSH_PROMISE frame.", decoder_.error_detail());
+}
+
+TEST_F(HttpDecoderTest, EmptyGoAwayFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "07"    // type (GOAWAY)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnError(&decoder_));
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_INVALID_FRAME_DATA, decoder_.error());
+  EXPECT_EQ("Unable to read GOAWAY stream_id", decoder_.error_detail());
+}
+
+TEST_F(HttpDecoderTest, EmptyMaxPushIdFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "0d"    // type (MAX_PUSH_ID)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnError(&decoder_));
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_INVALID_FRAME_DATA, decoder_.error());
+  EXPECT_EQ("Unable to read push_id", decoder_.error_detail());
+}
+
+TEST_F(HttpDecoderTest, EmptyDuplicatePushFrame) {
+  std::string input = QuicTextUtils::HexDecode(
+      "0e"    // type (DUPLICATE_PUSH)
+      "00");  // frame length
+
+  EXPECT_CALL(visitor_, OnError(&decoder_));
+  EXPECT_EQ(input.size(), ProcessInput(input));
+  EXPECT_EQ(QUIC_INVALID_FRAME_DATA, decoder_.error());
+  EXPECT_EQ("Unable to read push_id", decoder_.error_detail());
+}
 
 }  // namespace test
 

quic/core/http/quic_spdy_stream.cc

diff --git a/quic/core/http/quic_spdy_stream.cc b/quic/core/http/quic_spdy_stream.cc
index a407d5c..a089345 100644
--- a/quic/core/http/quic_spdy_stream.cc
+++ b/quic/core/http/quic_spdy_stream.cc
@@ -127,7 +127,7 @@
   bool OnPushPromiseFrameStart(PushId push_id,
                                QuicByteCount header_length,
                                QuicByteCount push_id_length) override {
-    if (!VersionHasStreamType(stream_->transport_version())) {
+    if (!VersionUsesQpack(stream_->transport_version())) {
       CloseConnectionOnWrongFrame("Push Promise");
       return false;
     }
@@ -880,6 +880,7 @@
 
 bool QuicSpdyStream::OnHeadersFramePayload(QuicStringPiece payload) {
   DCHECK(VersionUsesQpack(transport_version()));
+  DCHECK(qpack_decoded_headers_accumulator_);
 
   if (headers_decompressed_) {
     trailers_payload_length_ += payload.length();
@@ -904,6 +905,7 @@
 
 bool QuicSpdyStream::OnHeadersFrameEnd() {
   DCHECK(VersionUsesQpack(transport_version()));
+  DCHECK(qpack_decoded_headers_accumulator_);
 
   auto result = qpack_decoded_headers_accumulator_->EndHeaderBlock();