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

Issue 714367 link

Starred by 1 user

Issue metadata

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



Sign in to add a comment

Integer-overflow in net::SpdyStream::AdjustSendWindowSize

Project Member Reported by ClusterFuzz, Apr 22 2017

Issue description

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

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=465907:465934

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


Issue filed automatically.

See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.
 
Cc: msrchandra@chromium.org
Components: Internals>Network
Labels: M-60 Test-Predator-Wrong-CLs
Owner: diannahu@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL did not provide any possible suspects.
Using Code Search for the file, "spdy_stream.cc" assigning to the concern owner.
Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/401e364c7ce7bf260c825537c7f549b5d5fc6039

@diannahu -- Could you please look into the issue, kindly re-assign if this is not related to your changes.
Thank You.
Cc: b...@chromium.org
The linked commit involves changing a string type to a string alias; it looks unrelated to integer overflow.

Taking a look at the method SpdyStream::AdjustSendWindowSize() in question: https://cs.chromium.org/chromium/src/net/spdy/spdy_stream.cc?l=214&rcl=c4cb0d38f0bd26578120775e9c09be813b5215c0

It seems that we do check for wraparound/overflow but just DCHECK. We could potentially send a RST_STREAM for this stream-level overflow. CCing Bence for thoughts, thanks!

Comment 3 by b...@chromium.org, Apr 24 2017

Owner: b...@chromium.org
Re #2: Indeed we should send an RST_STREAM.  Since delta_window_size ultimately comes from the wire and there is no check agains too large values prior to this point, resetting the stream is the right way to handle this.  Thank you.
Project Member

Comment 4 by ClusterFuzz, Apr 25 2017

ClusterFuzz has detected this issue as fixed in range 465934:466800.

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

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=465907:465934
Fixed: https://clusterfuzz.com/revisions?job=libfuzzer_chrome_ubsan&range=465934:466800

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


See https://chromium.googlesource.com/chromium/src/+/master/testing/libfuzzer/reproducing.md for more information.

If you suspect that the result above is incorrect, try re-doing that job on the test case report page.
Project Member

Comment 5 by ClusterFuzz, Apr 25 2017

Labels: ClusterFuzz-Verified
Status: Verified (was: Assigned)
ClusterFuzz testcase 4647002106494976 is verified as fixed, so closing issue.

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.

Comment 6 by b...@chromium.org, Apr 25 2017

Labels: ClusterFuzz-Wrong
Status: Assigned (was: Verified)
Reopening.  I do not believe this bug has been fixed.  In particular, I do not see any changes to net/spdy in the "fixed" range 465934:466800 apart from https://codereview.chromium.org/2832973003 which is non-functional.
Status: WontFix (was: Assigned)
Bulk-WontFixing these bugs. This was a bug on ClusterFuzz side, see bug 717534. We will start seeing new testcases auto-filed in a day or two. We can't leave these open as ClusterFuzz won't autoverify them after ClusterFuzz-Wrong label.
Labels: -ClusterFuzz-Wrong
We have made a bunch of changes on ClusterFuzz side, so resetting ClusterFuzz-Wrong label.

Sign in to add a comment