google / quiche

BSD 3-Clause "New" or "Revised" License
660 stars 132 forks source link

IETF Connection Close Frame - Possible narrowing of error code coming from reason phrase #53

Closed kinokrt closed 1 year ago

kinokrt commented 1 year ago

Looks like error code coming from reason phrase of CLOSE_FRAME can be very big number (64 bits) but the target enum QuicErrorCode can be of non-fixed underlying type unsigned int (32 bits). This can lead to narrowing. I have checked this behavior by updating the QuicFramerTest.ConnectionCloseFrameWithUnknownErrorCode so it sends following reason phrase "4294967296:bla" which leads to quic_error_code with value 0 (QUIC_NO_ERROR) because of narrowing.

The problematic code:

void MaybeExtractQuicErrorCode(QuicConnectionCloseFrame* frame) {
  std::vector<absl::string_view> ed = absl::StrSplit(frame->error_details, ':');
  uint64_t extracted_error_code;

...
...
...

  frame->quic_error_code = static_cast<QuicErrorCode>(extracted_error_code);
}
bencebeky commented 1 year ago

Thank you for pointing out this issue. It has been fixed at https://github.com/google/quiche/commit/b7f167071e34a5f9834ff9bafdf1e62215f2138e.