commit 8dfeebd32d277fcfa50df99042d2b8ceeaecf369
author bnc <bnc@google.com> Tue Jul 16 11:41:29 2019 -0700
committer Copybara-Service <copybara-worker@google.com> Tue Jul 16 11:53:01 2019 -0700
tree 54f3fb68d23ce813161b8a8d61aa3a7a883dd9de
parent 733552e0b4fe96a298e0f9d3cff4842329f9091d

Make HttpDecoder::ParsePriorityFrame() more robust.

1. As https://crbug.com/981291 and https://crbug.com/981646 demonstrate,
   ParsePriorityFrame() used crashes on certain input.  Fix that.
2. Signal QUIC_INVALID_FRAME_DATA instead of QUIC_INTERNAL_ERROR, since
   handling invalid input is not an internal bug but an error on the
   sender's part.
3. Signal error if PRIORITY frame has superfluous payload.
4. Add tests.

gfe-relnote: n/a, change in QUIC v99-only class
PiperOrigin-RevId: 258409280
Change-Id: If2d5a14c222955ebaa4650b0723be180330c8d1a

quic/core/http/http_decoder.cc

diff --git a/quic/core/http/http_decoder.cc b/quic/core/http/http_decoder.cc
index 1c4c083..3388b2c 100644
--- a/quic/core/http/http_decoder.cc
+++ b/quic/core/http/http_decoder.cc
@@ -449,27 +449,42 @@
 bool HttpDecoder::ParsePriorityFrame(QuicDataReader* reader,
                                      PriorityFrame* frame) {
   uint8_t flags;
-  bool success = reader->ReadUInt8(&flags);
-  DCHECK(success);
+  if (!reader->ReadUInt8(&flags)) {
+    // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
+    RaiseError(QUIC_INVALID_FRAME_DATA, "Unable to read PRIORITY frame flags.");
+    return false;
+  }
 
   frame->prioritized_type =
       static_cast<PriorityElementType>(ExtractBits(flags, 2, 6));
   frame->dependency_type =
       static_cast<PriorityElementType>(ExtractBits(flags, 2, 4));
+  // TODO(b/137662729): Update bitmask for exclusive flag.
   frame->exclusive = flags % 2 == 1;
-  // TODO(bnc): Handle partial delivery.
+  // TODO(b/137359636): Handle partial delivery.
   if (frame->prioritized_type != ROOT_OF_TREE &&
       !reader->ReadVarInt62(&frame->prioritized_element_id)) {
-    RaiseError(QUIC_INTERNAL_ERROR, "Unable to read prioritized_element_id");
+    // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
+    RaiseError(QUIC_INVALID_FRAME_DATA,
+               "Unable to read prioritized_element_id.");
     return false;
   }
   if (frame->dependency_type != ROOT_OF_TREE &&
       !reader->ReadVarInt62(&frame->element_dependency_id)) {
-    RaiseError(QUIC_INTERNAL_ERROR, "Unable to read element_dependency_id");
+    // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
+    RaiseError(QUIC_INVALID_FRAME_DATA,
+               "Unable to read element_dependency_id.");
     return false;
   }
   if (!reader->ReadUInt8(&frame->weight)) {
-    RaiseError(QUIC_INTERNAL_ERROR, "Unable to read priority frame weight");
+    // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
+    RaiseError(QUIC_INVALID_FRAME_DATA,
+               "Unable to read priority frame weight.");
+    return false;
+  }
+  if (!reader->IsDoneReading()) {
+    // TODO(b/124216424): Use HTTP_MALFORMED_FRAME.
+    RaiseError(QUIC_INVALID_FRAME_DATA, "Superfluous data in priority frame.");
     return false;
   }
   return true;