New issue
Advanced search Search tips

Issue 871005 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Aug 9
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug-Security



Sign in to add a comment

Heap-use-after-free in views::Slider::SetValueInternal

Project Member Reported by ClusterFuzz, Aug 4

Issue description

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

Fuzzer: inferno_flicker
Job Type: linux_asan_chrome_chromeos
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60300046e0d8
Crash State:
  views::Slider::SetValueInternal
  ash::UnifiedSliderView::SetSliderValue
  chromeos::CrasAudioHandler::OutputNodeVolumeChanged
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=578960:578961

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

Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

Issue filed automatically.

See https://github.com/google/clusterfuzz-tools for more information.
 
Project Member

Comment 1 by ClusterFuzz, Aug 4

Components: Internals>Views UI>Shell>StatusArea
Labels: Test-Predator-Auto-Components
Automatically applying components based on crash stacktrace and information from OWNERS files.

If this is incorrect, please apply the Test-Predator-Wrong-Components label.
Project Member

Comment 2 by ClusterFuzz, Aug 4

Labels: Test-Predator-Auto-Owner
Owner: tetsui@chromium.org
Status: Assigned (was: Untriaged)
Automatically assigning owner based on suspected regression changelist https://chromium.googlesource.com/chromium/src/+/382a5c036c655ea2492d9119ef8ba9e9bb1c76c9 (Re-enable Alt-Shift-N for UnifiedSystemTray.).

If this is incorrect, please let us know why and apply the Test-Predator-Wrong-CLs label. If you aren't the correct owner for this issue, please unassign yourself as soon as possible so it can be re-triaged.
Project Member

Comment 3 by sheriffbot@chromium.org, Aug 4

Labels: Target-70 M-70
Project Member

Comment 4 by sheriffbot@chromium.org, Aug 4

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

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

Comment 5 by sheriffbot@chromium.org, Aug 4

Labels: Pri-1
Status: Started (was: Assigned)
Project Member

Comment 7 by bugdroid1@chromium.org, Aug 9

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

commit 76c395e0b40df47da90d0c4cdb81ba314ab1b7ec
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Aug 09 02:10:05 2018

Fix UnifiedVolumeView asan failure during closing.

This CL fixes UnifiedVolumeView asan failure when
1. the slider is in unified system tray main bubble
2. volume key is pressed during bubble fade out animation

This is because controllers are deleted before views are deleted. This
is intented because usually controllers are called only when a user
makes mouse event in the bubble. Slider is an exception because
it also notifies controller that value is changed by API although
it's ignored on controller side. Other possible fixes are:

* Make controller ptr in the view WeakPtr.
* UnifiedSliderView implement Slider listener and not forward changes
  caused by API.

The CL is the least verbose fix.

TEST=manual
BUG= 871005 

Change-Id: Ifb40b4107e1a04936bf6cd6e0649139498e15746
Reviewed-on: https://chromium-review.googlesource.com/1164962
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581759}
[modify] https://crrev.com/76c395e0b40df47da90d0c4cdb81ba314ab1b7ec/ash/system/unified/unified_slider_view.cc

Status: Fixed (was: Started)
Project Member

Comment 9 by bugdroid1@chromium.org, Aug 9

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

commit 78de4716b3d2ab19fd950d1b8352e5c7890bcd81
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Aug 09 06:48:07 2018

Revert "Fix UnifiedVolumeView asan failure during closing."

This reverts commit 76c395e0b40df47da90d0c4cdb81ba314ab1b7ec.

Reason for revert: Initial position of sliders are broken

Original change's description:
> Fix UnifiedVolumeView asan failure during closing.
> 
> This CL fixes UnifiedVolumeView asan failure when
> 1. the slider is in unified system tray main bubble
> 2. volume key is pressed during bubble fade out animation
> 
> This is because controllers are deleted before views are deleted. This
> is intented because usually controllers are called only when a user
> makes mouse event in the bubble. Slider is an exception because
> it also notifies controller that value is changed by API although
> it's ignored on controller side. Other possible fixes are:
> 
> * Make controller ptr in the view WeakPtr.
> * UnifiedSliderView implement Slider listener and not forward changes
>   caused by API.
> 
> The CL is the least verbose fix.
> 
> TEST=manual
> BUG= 871005 
> 
> Change-Id: Ifb40b4107e1a04936bf6cd6e0649139498e15746
> Reviewed-on: https://chromium-review.googlesource.com/1164962
> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581759}

TBR=yoshiki@chromium.org,tetsui@chromium.org

Change-Id: Ib02ac9cc17a77bc7c7fafe437f3d289e2033ce2b
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug:  871005 
Reviewed-on: https://chromium-review.googlesource.com/1168862
Reviewed-by: Tetsui Ohkubo <tetsui@chromium.org>
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581812}
[modify] https://crrev.com/78de4716b3d2ab19fd950d1b8352e5c7890bcd81/ash/system/unified/unified_slider_view.cc

Status: Started (was: Fixed)
Project Member

Comment 11 by bugdroid1@chromium.org, Aug 9

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

commit a39d74756dea571186387403cf5e6f9f7243c820
Author: Tetsui Ohkubo <tetsui@chromium.org>
Date: Thu Aug 09 10:16:34 2018

Reland "Fix UnifiedVolumeView asan failure during closing."

This is a reland of 76c395e0b40df47da90d0c4cdb81ba314ab1b7ec

Reason for reland: slider initial position was broken.

Original change's description:
> Fix UnifiedVolumeView asan failure during closing.
>
> This CL fixes UnifiedVolumeView asan failure when
> 1. the slider is in unified system tray main bubble
> 2. volume key is pressed during bubble fade out animation
>
> This is because controllers are deleted before views are deleted. This
> is intented because usually controllers are called only when a user
> makes mouse event in the bubble. Slider is an exception because
> it also notifies controller that value is changed by API although
> it's ignored on controller side. Other possible fixes are:
>
> * Make controller ptr in the view WeakPtr.
> * UnifiedSliderView implement Slider listener and not forward changes
>   caused by API.
>
> The CL is the least verbose fix.
>
> TEST=manual
> BUG= 871005 
>
> Change-Id: Ifb40b4107e1a04936bf6cd6e0649139498e15746
> Reviewed-on: https://chromium-review.googlesource.com/1164962
> Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
> Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#581759}

Bug:  871005 
Change-Id: I24691cfe371e08251ed13c24b3a60db1e1b2cb9f
Reviewed-on: https://chromium-review.googlesource.com/1168882
Commit-Queue: Tetsui Ohkubo <tetsui@chromium.org>
Reviewed-by: Yoshiki Iguchi <yoshiki@chromium.org>
Cr-Commit-Position: refs/heads/master@{#581850}
[modify] https://crrev.com/a39d74756dea571186387403cf5e6f9f7243c820/ash/system/unified/unified_slider_view.cc

Status: Fixed (was: Started)
Project Member

Comment 13 by sheriffbot@chromium.org, Aug 9

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Project Member

Comment 14 by ClusterFuzz, Aug 10

ClusterFuzz has detected this issue as fixed in range 581849:581850.

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

Fuzzer: inferno_flicker
Job Type: linux_asan_chrome_chromeos
Platform Id: linux

Crash Type: Heap-use-after-free READ 8
Crash Address: 0x60300046e0d8
Crash State:
  views::Slider::SetValueInternal
  ash::UnifiedSliderView::SetSliderValue
  chromeos::CrasAudioHandler::OutputNodeVolumeChanged
  
Sanitizer: address (ASAN)

Recommended Security Severity: High

Regressed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=578960:578961
Fixed: https://clusterfuzz.com/revisions?job=linux_asan_chrome_chromeos&range=581849:581850

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

Additional requirements: Requires Gestures

Additional requirements: Requires HTTP

See https://github.com/google/clusterfuzz-tools 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 15 by ClusterFuzz, Aug 10

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

If this is incorrect, please add ClusterFuzz-Wrong label and re-open the issue.
Labels: -ReleaseBlock-Stable
Project Member

Comment 17 by sheriffbot@chromium.org, Nov 15

Labels: -Restrict-View-SecurityNotify allpublic
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

Sign in to add a comment