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

Issue 881958 link

Starred by 0 users

Issue metadata

Status: Fixed
Owner:
Closed: Sep 21
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 1
Type: Bug

Blocking:
issue 886903


Participants' hotlists:
Better-Together-Launch-Blockers


Sign in to add a comment

More robustly verify ProximityAuthProfilePrefManager local state before persisting to local state

Project Member Reported by hansberry@chromium.org, Sep 7

Issue description

In debug builds, ProximityAuthProfilePrefManager's local_state_ pointer may be initialized to garbage memory (not nullptr), causing this check [1] to incorrectly pass as true.

The check in question should also verify the validity of account_id_.

This is not a major issue because it only affects debug builds.

1) https://cs.chromium.org/chromium/src/chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.cc?sq=package:chromium&dr=CSs&g=0&l=198
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 11

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

commit 4009dbbbb2d92e5ac0998991181d136fae15ca52
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Tue Sep 11 17:43:38 2018

Verify ProximityAuthProfilePrefManager local state before persisting to it.

The local_state_ pointer was initializing to garbage memory when Chrome was run
in debug mode, causing a check if local_state_ == nullptr to incorrectly return false.
leading to a crash. This issue was preventing developers from running Chrome in debug
mode if Smart Lock is enabled.

Bug:  881958 
Change-Id: Ia796eefe244ca17a1905e489aeee9a310828802c
Reviewed-on: https://chromium-review.googlesource.com/1218442
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590379}
[modify] https://crrev.com/4009dbbbb2d92e5ac0998991181d136fae15ca52/chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.cc
[modify] https://crrev.com/4009dbbbb2d92e5ac0998991181d136fae15ca52/chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.h

Status: Fixed (was: Assigned)
Blocking: 886903
Labels: -Pri-2 -M-71 Merge-Request-70 M-70 Pri-1
Status: Started (was: Fixed)
I was incorrect that this only affects debug builds -- we are seeing it on dev-channel. Requesting merge to M70.
Project Member

Comment 5 by sheriffbot@chromium.org, Sep 19

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
Labels: -Merge-Approved-70 Merge-Merged-70-refsbranch-heads3538
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d13274c5bc1a2c1f598d662bb20253b17eceffc4
Commit: d13274c5bc1a2c1f598d662bb20253b17eceffc4
Author: hansberry@chromium.org
Commiter: hansberry@chromium.org
Date: 2018-09-21 17:32:40 +0000 UTC
Verify ProximityAuthProfilePrefManager local state before persisting to it.

The local_state_ pointer was initializing to garbage memory when Chrome was run
in debug mode, causing a check if local_state_ == nullptr to incorrectly return false.
leading to a crash. This issue was preventing developers from running Chrome in debug
mode if Smart Lock is enabled.

Bug:  881958 
Change-Id: Ia796eefe244ca17a1905e489aeee9a310828802c
Reviewed-on: https://chromium-review.googlesource.com/1218442
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590379}(cherry picked from commit 4009dbbbb2d92e5ac0998991181d136fae15ca52)
Reviewed-on: https://chromium-review.googlesource.com/1239153
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#560}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
Project Member

Comment 8 by bugdroid1@chromium.org, Sep 21

Labels: merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d13274c5bc1a2c1f598d662bb20253b17eceffc4

commit d13274c5bc1a2c1f598d662bb20253b17eceffc4
Author: Ryan Hansberry <hansberry@chromium.org>
Date: Fri Sep 21 17:32:40 2018

Verify ProximityAuthProfilePrefManager local state before persisting to it.

The local_state_ pointer was initializing to garbage memory when Chrome was run
in debug mode, causing a check if local_state_ == nullptr to incorrectly return false.
leading to a crash. This issue was preventing developers from running Chrome in debug
mode if Smart Lock is enabled.

Bug:  881958 
Change-Id: Ia796eefe244ca17a1905e489aeee9a310828802c
Reviewed-on: https://chromium-review.googlesource.com/1218442
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Commit-Queue: Ryan Hansberry <hansberry@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590379}(cherry picked from commit 4009dbbbb2d92e5ac0998991181d136fae15ca52)
Reviewed-on: https://chromium-review.googlesource.com/1239153
Reviewed-by: Ryan Hansberry <hansberry@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#560}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/d13274c5bc1a2c1f598d662bb20253b17eceffc4/chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.cc
[modify] https://crrev.com/d13274c5bc1a2c1f598d662bb20253b17eceffc4/chromeos/components/proximity_auth/proximity_auth_profile_pref_manager.h

Status: Fixed (was: Started)

Sign in to add a comment