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

Issue 753222 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Aug 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: Linux
Pri: 1
Type: Bug



Sign in to add a comment

Null-dereference READ in base::DeserializeHistogramInfo

Project Member Reported by ClusterFuzz, Aug 8 2017

Issue description

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

Fuzzer: ipc_fuzzer_mut
Job Type: linux_asan_chrome_ipc
Platform Id: linux

Crash Type: Null-dereference READ
Crash Address: 0x000000000028
Crash State:
  base::DeserializeHistogramInfo
  base::HistogramDeltaSerialization::DeserializeAndAddSamples
  content::HistogramSynchronizer::OnHistogramDataCollected
  
Sanitizer: address (ASAN)

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_ipc&range=492335:492340

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


Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Cc: msrchandra@chromium.org
Labels: Test-Predator-Wrong-CLs M-62
Owner: bcwh...@chromium.org
Status: Assigned (was: Untriaged)
Predator and CL could not provide any possible suspects.
Using Code Search for the file, "histogram_base.cc" assigning to concern owner.

Suspecting Commit#
https://chromium.googlesource.com/chromium/src/+/2805edaba1b852b6ce489a8f88bd8e90e51b73d2

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

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 9 2017

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

commit 7eb91483f6c48c26d07656237f8a2a2ef4f5a85d
Author: Brian White <bcwhite@chromium.org>
Date: Wed Aug 09 19:54:45 2017

Handle failed histogram creation during deserialization.

Also: change some NULL values to nullptr.

Bug:  753222 
Change-Id: Ib8621abb284138c5d3c0f31e24de2e8181a906ba
Reviewed-on: https://chromium-review.googlesource.com/606768
Commit-Queue: Brian White <bcwhite@chromium.org>
Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#493110}
[modify] https://crrev.com/7eb91483f6c48c26d07656237f8a2a2ef4f5a85d/base/metrics/histogram.cc

Status: Fixed (was: Assigned)
Labels: Merge-Request-61
Project Member

Comment 5 by sheriffbot@chromium.org, Aug 14 2017

Labels: -Merge-Request-61 Merge-Review-61 Hotlist-Merge-Review
This bug requires manual review: M61 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), ketakid@(ChromeOS), govind@(Desktop)

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

Comment 6 by gov...@chromium.org, Aug 14 2017

Before we approve merge to M61, please answer followings:
* Is this M61 regression? Is it critical?
* Is the change well baked/verified in Canary, having enough automation tests coverage and safe to merge to M61?
* Any other important details to justify the merge.

Please note M61 is already in Beta, so merge bar is very high. Thank you.
It protects against a possible browser crash caused by a compromised renderer that manages to send an invalid serialized histogram.  It was found by the fuzzer.

There is no known instance of this occurring in the wild so it may not be a priority.

Comment 8 by gov...@chromium.org, Aug 14 2017

Cc: abdulsyed@chromium.org
Labels: -Merge-Review-61 Merge-Rejected-61
Rejecting merge to M61 based on comment #7 "There is no known instance of this occurring in the wild so it may not be a priority". Please let me know if there is any concern here.

+abdulsyed@
Project Member

Comment 9 by ClusterFuzz, Aug 16 2017

Labels: Needs-Feedback
ClusterFuzz testcase 6480695710187520 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 10 Deleted

Comment 11 Deleted

Status: Started (was: Fixed)
Project Member

Comment 13 by bugdroid1@chromium.org, Aug 21 2017

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

commit 82027ff5dafbb3fbafe44fede998a33667ea2bd1
Author: Brian White <bcwhite@chromium.org>
Date: Mon Aug 21 19:50:22 2017

Handle invalid histograms during serialization.

If the incoming data is not valid, no histogram will be created a
nullptr will be returned.  Trying to do further validation on that
pointer would then crash.

Bug:  753222 
Change-Id: I91c324a2e56e2039dd2f0726fa42f0356e3991bb
Reviewed-on: https://chromium-review.googlesource.com/619490
Reviewed-by: Alexei Svitkine (very slow) <asvitkine@chromium.org>
Commit-Queue: Brian White <bcwhite@chromium.org>
Cr-Commit-Position: refs/heads/master@{#496032}
[modify] https://crrev.com/82027ff5dafbb3fbafe44fede998a33667ea2bd1/base/metrics/histogram.cc

Status: Fixed (was: Started)

Sign in to add a comment