New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 732019 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

SpdyStream::AdjustSendWindowSize should reset connection upon overflow.

Project Member Reported by ClusterFuzz, Jun 10 2017

Issue description

Detailed report: https://clusterfuzz.com/testcase?key=5400455256735744

Fuzzer: libFuzzer_net_spdy_session_fuzzer
Job Type: libfuzzer_chrome_ubsan
Platform Id: linux

Crash Type: Integer-overflow
Crash Address: 
Crash State:
  net::SpdyStream::AdjustSendWindowSize
  net::SpdySession::UpdateStreamsSendWindowSize
  net::SpdySession::HandleSetting
  
Sanitizer: undefined (UBSAN)

Regressed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=478353:478423

Reproducer Testcase: https://clusterfuzz.com/download?testcase_id=5400455256735744


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org kkaluri@chromium.org
Components: Blink>Network>StreamsAPI
Labels: M-63 Test-Predator-Wrong-CLs
Owner: b...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "spdy_session.cc" assigning to the concern owner who might be related or worked on similar file.

bnc@ -- Could you please look into the issue, kindly re-assign if this is not related to your changes.


Thank You.

Comment 2 by b...@chromium.org, Sep 14 2017

Components: -Blink>Network>StreamsAPI Internals>Network>HTTP2
Chrome should handle this case by closing the stream.  According to RFC7540 Section 6.9.1: "A sender MUST NOT allow a flow-control window to exceed 2^31-1 octets. If a sender receives a WINDOW_UPDATE that causes a flow-control window to exceed this maximum, it MUST terminate either the stream or the connection, as appropriate. For streams, the sender sends a RST_STREAM with an error code of FLOW_CONTROL_ERROR; for the connection, a GOAWAY frame with an error code of FLOW_CONTROL_ERROR is sent."

Comment 3 by b...@chromium.org, Sep 14 2017

Status: Started (was: Assigned)
Oops, that case is correctly handled by SpdyStream::IncreaseSendWindowSize().  The relevant text is the last paragraph of Section 6.9.2:
"An endpoint MUST treat a change to SETTINGS_INITIAL_WINDOW_SIZE that causes any flow-control window to exceed the maximum size as a connection error (Section 5.4.1) of type FLOW_CONTROL_ERROR."

Comment 4 by b...@chromium.org, Sep 14 2017

Summary: SpdyStream::AdjustSendWindowSize should reset connection upon overflow. (was: Integer-overflow in net::SpdyStream::AdjustSendWindowSize)
Project Member

Comment 5 by bugdroid1@chromium.org, Sep 15 2017

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

commit 05dcb038be9f06f8b8363978578d6be5ac7208fb
Author: Bence Béky <bnc@chromium.org>
Date: Fri Sep 15 20:07:58 2017

Reset connection if SETTINGS_INITIAL_WINDOW_SIZE causes overflow.

Reset HTTP/2 connection if new SETTINGS_INITIAL_WINDOW_SIZE value causes
overflow of any stream flow control window.  This is to comply with
RFC7540, Section 6.9.2, last paragraph.

BUG= 732019 

Change-Id: I489f3251e00b2ef2a110a798b9e5ff092e51e19f
Reviewed-on: https://chromium-review.googlesource.com/667542
Commit-Queue: Bence Béky <bnc@chromium.org>
Reviewed-by: Helen Li <xunjieli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#502342}
[modify] https://crrev.com/05dcb038be9f06f8b8363978578d6be5ac7208fb/net/spdy/chromium/spdy_network_transaction_unittest.cc
[modify] https://crrev.com/05dcb038be9f06f8b8363978578d6be5ac7208fb/net/spdy/chromium/spdy_session.cc
[modify] https://crrev.com/05dcb038be9f06f8b8363978578d6be5ac7208fb/net/spdy/chromium/spdy_stream.cc
[modify] https://crrev.com/05dcb038be9f06f8b8363978578d6be5ac7208fb/net/spdy/chromium/spdy_stream.h
[modify] https://crrev.com/05dcb038be9f06f8b8363978578d6be5ac7208fb/net/spdy/chromium/spdy_stream_unittest.cc

Comment 6 by b...@chromium.org, Sep 16 2017

Status: Fixed (was: Started)
Project Member

Comment 7 by ClusterFuzz, Sep 24 2017

Labels: Needs-Feedback
ClusterFuzz testcase 5400455256735744 is still reproducing on tip-of-tree build (trunk).

Please re-test your fix against this testcase and if the fix was incorrect or incomplete, please re-open the bug. Otherwise, ignore this notification and add ClusterFuzz-Wrong label.

Comment 8 by b...@chromium.org, Sep 26 2017

Labels: -Needs-Feedback ClusterFuzz-Wrong
I can reproduce the crash at 05dcb038be~ (right before the fix above) and I can confirm that the crash does not reproduce at 05dcb038be (the fix), nor at 9b0497eba5 (current tip-of-tree).  Therefore I believe that comment #7 is incorrect.
Project Member

Comment 9 by ClusterFuzz, Oct 1 2017

Labels: Test-Predator-AutoComponents
Automatically applying components based on information from OWNERS files. If this seems incorrect, please apply the Test-Predator-Wrong-Components label.

Comment 10 by b...@chromium.org, Oct 2 2017

Issue 770570 has been merged into this issue.
Labels: -Test-Predator-AutoComponents Test-Predator-Auto-Components

Sign in to add a comment