New issue
Advanced search Search tips

Issue 660617 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 2
Type: Bug



Sign in to add a comment

Undefined-shift in net::HttpServerPropertiesImpl::MarkAlternativeServiceBroken

Project Member Reported by ClusterFuzz, Oct 29 2016

Issue description

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5462970399457280

Fuzzer: inferno_webbot
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  net::HttpServerPropertiesImpl::MarkAlternativeServiceBroken
  net::HttpServerPropertiesManager::MarkAlternativeServiceBroken
  net::HttpStreamFactoryImpl::JobController::ReportBrokenAlternativeService
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=390670:390702

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95CFCxFDXsFDuWocBrSG-ONayCQ1_R1BOtE895-yamX6KgvxXgqfRo4Y0hU8wexNVwNKu5pL7x1T6-KoquDB0Y3YY5N297HuecLOVm4s5uDhrOQHZ83BNIcDYxty7M0_QN-X6ZFD8yLnVodPw63QmOEuCB6rA?testcase_id=5462970399457280


Issue filed automatically.

See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs for more information.
 
Cc: tzik@chromium.org
Owner: zhongyi@chromium.org
Status: Assigned (was: Untriaged)
zhongyi@ could you please look into this.please feel free to re-assigned back if needed. thanks in advance !
Cc: b...@chromium.org
Owner: ----
Status: Available (was: Assigned)
runtime error: shift exponent 32 is too large for 32-bit type int

  int count = ++recently_broken_alternative_services_[alternative_service];
  base::TimeDelta delay =
      base::TimeDelta::FromSeconds(kBrokenAlternativeProtocolDelaySecs);
  base::TimeTicks when = base::TimeTicks::Now() + delay * (1 << (count - 1));

I think the invalid shift is coming from the (count - 1) exceeding 32 (actually we should limit within [0, 23] as delay is ~2^8).  

bnc@: could you take a look and confirm? Thanks!
Components: Internals>Network>QUIC
Though this is indeed a potentially overflow bug but it shouldn't happen in reality as the the overflow is caused by the alternative service to be marked as broken for 30 times already. Due to exponential backoff, this would mean the most recent alternative service has been marked as broken for ~34 yrs and it's timed out leading to the new time out for shift of 32 bits. Note this is only for fuzz test, this matches the prescription that it wouldn't happen in reality. 

Comment 4 by b...@chromium.org, Nov 2 2016

Owner: b...@chromium.org
Status: Started (was: Available)
Project Member

Comment 5 by bugdroid1@chromium.org, Nov 7 2016

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

commit 3c0589bd98e3022091ee6fd899fe218182424ccc
Author: bnc <bnc@chromium.org>
Date: Mon Nov 07 21:44:31 2016

Limit exponential backoff for broken alternative services.

This is to limit overflow in shift.

BUG= 660617 

Review-Url: https://codereview.chromium.org/2464263003
Cr-Commit-Position: refs/heads/master@{#430382}

[modify] https://crrev.com/3c0589bd98e3022091ee6fd899fe218182424ccc/net/http/http_server_properties_impl.cc

Project Member

Comment 6 by ClusterFuzz, Nov 8 2016

ClusterFuzz has detected this issue as fixed in range 430376:430398.

Detailed report: https://cluster-fuzz.appspot.com/testcase?key=5462970399457280

Fuzzer: inferno_webbot
Job Type: linux_ubsan_chrome
Platform Id: linux

Crash Type: Undefined-shift
Crash Address: 
Crash State:
  net::HttpServerPropertiesImpl::MarkAlternativeServiceBroken
  net::HttpServerPropertiesManager::MarkAlternativeServiceBroken
  net::HttpStreamFactoryImpl::JobController::ReportBrokenAlternativeService
  
Regressed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=390670:390702
Fixed: https://cluster-fuzz.appspot.com/revisions?job=linux_ubsan_chrome&range=430376:430398

Unminimized Testcase: https://cluster-fuzz.appspot.com/download/AMIfv95CFCxFDXsFDuWocBrSG-ONayCQ1_R1BOtE895-yamX6KgvxXgqfRo4Y0hU8wexNVwNKu5pL7x1T6-KoquDB0Y3YY5N297HuecLOVm4s5uDhrOQHZ83BNIcDYxty7M0_QN-X6ZFD8yLnVodPw63QmOEuCB6rA?testcase_id=5462970399457280


See https://dev.chromium.org/Home/chromium-security/bugs/reproducing-clusterfuzz-bugs 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 7 by ClusterFuzz, Nov 8 2016

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

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

Comment 8 by b...@chromium.org, Nov 8 2016

Re #3: I agree that this should not be an issue in reality.  I'm fixing it anyway, not only to make the fuzzer happy, but also because backing off from using QUIC for more than a few days is unreasonable: a single failed QUIC request  every few days is a very small price to pay for potentially discovering that QUIC works and getting the latency improvements.
Project Member

Comment 9 by sheriffbot@chromium.org, Nov 22 2016

Labels: -Restrict-View-EditIssue
Removing EditIssue view restrictions from ClusterFuzz filed bugs. If you believe that this issue should still be restricted, please reapply the label.

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

Sign in to add a comment