New issue
Advanced search Search tips

Issue 911855 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 7
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug
Team-Security-UX



Sign in to add a comment

Crash in EULA screen with committed interstitials.

Project Member Reported by carlosil@chromium.org, Dec 4

Issue description

If an interstitial is triggered in the EULA webview shown on the Chrome OS OOBE (for example due to a captive portal), the load is aborted. This can trigger a crash, due to race conditions causing the Browser Context to be null in SSLErrorHandler::HandleSSLError.

 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 5

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

commit 4498310486ff183b27eb250a5ffe3a8ce7325dc6
Author: Carlos IL <carlosil@chromium.org>
Date: Wed Dec 05 19:01:19 2018

Add check for profile in HandleSSLError.

There are edge cases in which HandleSSLError can be called without a
browser context, make sure it returns instead of crashing. This was
causing a crash with committed interstitials enabled if an
interstitial was shown in the EULA screen on the first run.

Bug:911855

Change-Id: I030eb7269c1a3ed1388d159a6cfea069c09d0532
Reviewed-on: https://chromium-review.googlesource.com/c/1359827
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/heads/master@{#614047}
[modify] https://crrev.com/4498310486ff183b27eb250a5ffe3a8ce7325dc6/chrome/browser/ssl/ssl_error_handler.cc

Labels: -Pri-3 M-72 Merge-Request-72 Pri-2
Requesting merge approval for 72, though the bug is relatively rare, considering the fix is very low risk (add a missing null check), it'd be nice to have it fixed before launching.
Project Member

Comment 3 by sheriffbot@chromium.org, Dec 7

Labels: -Merge-Request-72 Hotlist-Merge-Approved Merge-Approved-72
Your change meets the bar and is auto-approved for M72. Please go ahead and merge the CL to branch 3626 manually. Please contact milestone owner if you have questions.
Owners: govind@(Android), kariahda@(iOS), djmm@(ChromeOS), abdulsyed@(Desktop)

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

Comment 4 by bugdroid1@chromium.org, Dec 7

Labels: -merge-approved-72 merge-merged-3626
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9658fae1bde3433ca09ed4da6b3b31c32b637888

commit 9658fae1bde3433ca09ed4da6b3b31c32b637888
Author: Carlos IL <carlosil@chromium.org>
Date: Fri Dec 07 19:50:51 2018

Add check for profile in HandleSSLError.

There are edge cases in which HandleSSLError can be called without a
browser context, make sure it returns instead of crashing. This was
causing a crash with committed interstitials enabled if an
interstitial was shown in the EULA screen on the first run.

Bug:911855

Change-Id: I030eb7269c1a3ed1388d159a6cfea069c09d0532
Reviewed-on: https://chromium-review.googlesource.com/c/1359827
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614047}(cherry picked from commit 4498310486ff183b27eb250a5ffe3a8ce7325dc6)
Reviewed-on: https://chromium-review.googlesource.com/c/1368445
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#147}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}
[modify] https://crrev.com/9658fae1bde3433ca09ed4da6b3b31c32b637888/chrome/browser/ssl/ssl_error_handler.cc

Status: Fixed (was: Started)
Labels: Merge-Merged-72-3626
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/9658fae1bde3433ca09ed4da6b3b31c32b637888

Commit: 9658fae1bde3433ca09ed4da6b3b31c32b637888
Author: carlosil@chromium.org
Commiter: carlosil@chromium.org
Date: 2018-12-07 19:50:51 +0000 UTC

Add check for profile in HandleSSLError.

There are edge cases in which HandleSSLError can be called without a
browser context, make sure it returns instead of crashing. This was
causing a crash with committed interstitials enabled if an
interstitial was shown in the EULA screen on the first run.

Bug:911855

Change-Id: I030eb7269c1a3ed1388d159a6cfea069c09d0532
Reviewed-on: https://chromium-review.googlesource.com/c/1359827
Reviewed-by: Mustafa Emre Acer <meacer@chromium.org>
Commit-Queue: Carlos IL <carlosil@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#614047}(cherry picked from commit 4498310486ff183b27eb250a5ffe3a8ce7325dc6)
Reviewed-on: https://chromium-review.googlesource.com/c/1368445
Reviewed-by: Carlos IL <carlosil@chromium.org>
Cr-Commit-Position: refs/branch-heads/3626@{#147}
Cr-Branched-From: d897fb137fbaaa9355c0c93124cc048824eb1e65-refs/heads/master@{#612437}

Sign in to add a comment