Thanks for the high quality bug report!

The behavior's weird, but we've mostly kept it because of inertia (And it's probably what FireFox used to do).  This sounds like a real potential attack vector to me - not one that's so important that we need to rush out a fix, but one that makes this weirdness definitely worth fixing, sooner rather than later.

Also, this doesn't cause issues with Chrome's protection - safebrowsing checks are done at the URL layer, and basically all code in Chrome beyond the HTTP stack sees the actual response body, after decompression (Since that's built into the network layer).  When we trigger the OSes download checks, those either use the downloaded file path, or would be passed the extracted stream (Not sure which - not familiar with that code).

The main threat model here is that a server or proxy may have security-related checks that only inspect the gzipped data, ignoring any extra bits at the end of the file.

Ah I see, Thanks for addressing this though. Caused some confusion until we figured out it was chrome. Hope this gets updated and fixed up though.

Tentatively assigning security labels.

This was done to match Mozilla's behavior at least according to the comment on the old code.

https://chromium.googlesource.com/chromium/src/+/fb4e32fc88050cd82bdb74edc90fb4fe3dfe56e7/net/filter/gzip_filter.cc

  if (decoding_status_ == DECODING_DONE) {
    if (GZIP_GET_INVALID_HEADER != gzip_header_status_)
      SkipGZipFooter();
    // Some server might send extra data after the gzip footer. We just copy
    // them out. Mozilla does this too.
    return CopyOut(dest_buffer, dest_len);
  }


GzipSourceStream (a replacement of GzipFilter) kept the (passing through data after EOF) behavior.

Experimentally, FireFox no longer seems to do this, so we seem to be the only ones doing it.  Even if we weren't, it's weird enough behavior that it would probably be best to push on removing it.

SGTM. Thanks for taking this!

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9cb5fa1df50d2ee343a40627d34a57f7486e6c1e

commit 9cb5fa1df50d2ee343a40627d34a57f7486e6c1e
Author: Matt Menke <mmenke@chromium.org>
Date: Wed Apr 18 18:22:26 2018

If there are extra bytes at the end of a gzip response, ignore them.

Previously, they were passed through unaltered, which was rather
unexpected. We could error out requests instead, but erroring out after
successfully decoding the response body seems likely to lead to more
compatibility issues than fix anything.

Bug:  833729 
Change-Id: I7e0948aa4d3b963f27f4c954d9576b89e963fffb
Reviewed-on: https://chromium-review.googlesource.com/1015516
Reviewed-by: Helen Li <xunjieli@chromium.org>
Commit-Queue: Matt Menke <mmenke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#551753}
[modify] https://crrev.com/9cb5fa1df50d2ee343a40627d34a57f7486e6c1e/net/filter/gzip_source_stream.cc
[modify] https://crrev.com/9cb5fa1df50d2ee343a40627d34a57f7486e6c1e/net/filter/gzip_source_stream.h
[modify] https://crrev.com/9cb5fa1df50d2ee343a40627d34a57f7486e6c1e/net/filter/gzip_source_stream_unittest.cc

I don't think this is worth a merge.  I would classify it as Sec-Severity-None, and think we should remove access restrictions, but I'll defer to others on that.

Hi v3rmies@ - I'm afraid the VRP panel declined to award for this, as we couldn't see how it could be exploited. Many thanks for the report though!

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot