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

Issue 598047 link

Starred by 0 users

Issue metadata

Status: Verified
Owner:
Closed: Apr 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS
Pri: 1
Type: Bug-Security



Sign in to add a comment

Address bar not updated when returning from network error page.

Project Member Reported by srikanthg@chromium.org, Mar 25 2016

Issue description

App Version: 51.0.2690.0 canary
iOS Version: 9.2.1, 9.3
Device: iPhone6s
URL: http://output.jsbin.com/fozaqiceve

This bug a little variant from previously reported http://crbug/597068 

Steps to reproduce:
  1. Launch Google Chrome canary
  2. Goto the above URL (http://output.jsbin.com/fozaqiceve)
  3. Tap on "Run test case"
  4. Observe that network error page is displayed.
  5. Tap on "Back to safety"

Observed results: Observe that url is still showing "https://tv.eurosport.com"

Expected results: URL should be updated to about:blank on tapping "Back to Safety"

Number of times you were able to reproduce: 5/5
Bug reproducible after clean install: Yes
Bug reproducible after clearing cache and cookies: Yes
Bug reproducible on Chrome Mobile on Android: Not tested
Bug reproducible on Dolphin/Safari/Firefox: Firefox: No, Safari: NO (Safari shows alert to continue/cancel, FF shows Error page and no option to Back)
Bug reproducible on current stable build (App Version, iOS Version): M49 YES
Bug reproducible on the current beta channel build (App Version, iOS Version): M50 YES

Link to video/image: https://drive.google.com/a/google.com/file/d/0B-xmXLQhjeKuVVBlOE5qUGZ0ZjA/preview 
 
Labels: ReleaseBlock-Stable M-50
JSBin output expired after few days, so moved test case URL to below:
http://blingtestapp.appspot.com/eurosport.html
Cc: carusom@chromium.org
We should include this into M50 respin, but it's too late to include into regular release as the change is risky.

CL1: https://chromereviews.googleplex.com/393757013/
CL2: https://codereview.chromium.org/1844363004/
Status: Started (was: Assigned)
Cc: palmer@chromium.org rohitrao@chromium.org
Components: Mobile>WebView>Glue
Labels: Security
This bug has impact on security as it allows spoofing *any* URL during MITM attack. SSL lock will be red and broken, but many users don't care about lock color.

The fix is risky because web// has never signaled *did finish load* for cases when reload happened with Empty Navigation Manager. So the change can potentially introduce the new crashes. I would be comfortable merging this to the branch only if the changes live on Canary for let's say a week. This is why I proposed to include the changes in respin.

Chris, how bad is this bug? Can this wait until M51?
Cc: f...@chromium.org
Components: Security>UX
Labels: -Restrict-View-Google -Pri-2 Security_Severity-Medium Security_Impact-Stable Restrict-View-SecurityNotify Pri-1
I'd rather not wait, this is a pretty troubling Omnibox spoof, and changes the apparent meaning of "Back To Safety". +felt to see if she agrees with the labels.
Project Member

Comment 7 by bugdroid1@chromium.org, Apr 4 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/4e709c3b7a1ebc7545725425ea088981f88cd4d0

commit 4e709c3b7a1ebc7545725425ea088981f88cd4d0
Author: eugenebut <eugenebut@google.com>
Date: Mon Apr 04 22:26:10 2016

Project Member

Comment 8 by bugdroid1@chromium.org, Apr 5 2016

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

commit 8466be358c11657cb90fe148c10017573e6cabdc
Author: eugenebut <eugenebut@chromium.org>
Date: Tue Apr 05 02:15:59 2016

[ios] Signal DidFinishLoad if target URL is invalid.

Target URL can be invalid if Reload is called for empty navigation
manager. Empty Navigation Manager is a reasonable state when the only
pending item was a page with badd SSL cert and user Tapped Go Back.

BUG= 598047 

Review URL: https://codereview.chromium.org/1844363004

Cr-Commit-Position: refs/heads/master@{#385085}

[modify] https://crrev.com/8466be358c11657cb90fe148c10017573e6cabdc/ios/web/web_state/ui/crw_web_controller.mm

Labels: Merge-Request-50

Comment 10 by tin...@google.com, Apr 5 2016

Labels: -Merge-Request-50 Merge-Review-50 Hotlist-Merge-Review
[Automated comment] Less than 2 weeks to go before AppStore submit on M50, manual review required.
Verified on iPhone6s, iPad Air2 with M51.0.2701.0 canary.
Tapping on "Back to Safety" redirects correctly to about:blank page. (document.write works/allowed on about:blank)

eugenebut@ you mentioned about potential new crashes with this change in comment#5. Please let us know if we need to run any regression tests, and if so, then what would be the areas need tobe focused on.
Status: Fixed (was: Started)
>> eugenebut@ you mentioned about potential new crashes with this change in comment#5. Please let us know if we need to run any regression tests, and if so, then what would be the areas need tobe focused on.

Prerequisite for the crashes can be following STR from this bug.
Let's cherrypick this and monitor the next beta release for crashes that could be caused by this change.
Project Member

Comment 14 by bugdroid1@chromium.org, Apr 5 2016

Labels: Merge-Merged-2661
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/382aab43feab834896fe50a6c25b4947356fdc43

commit 382aab43feab834896fe50a6c25b4947356fdc43
Author: eugenebut <eugenebut@google.com>
Date: Mon Apr 04 22:26:10 2016

Project Member

Comment 15 by bugdroid1@chromium.org, Apr 5 2016

The following revision refers to this bug:
  https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/382aab43feab834896fe50a6c25b4947356fdc43

commit 382aab43feab834896fe50a6c25b4947356fdc43
Author: eugenebut <eugenebut@google.com>
Date: Mon Apr 04 22:26:10 2016

Project Member

Comment 16 by bugdroid1@chromium.org, Apr 5 2016

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

commit b527df8f6c0fe8da64f67b58143ee34c4e6133be
Author: Eugene But <eugenebut@google.com>
Date: Tue Apr 05 21:46:18 2016

[ios] Signal DidFinishLoad if target URL is invalid.

Target URL can be invalid if Reload is called for empty navigation
manager. Empty Navigation Manager is a reasonable state when the only
pending item was a page with badd SSL cert and user Tapped Go Back.

BUG= 598047 

Review URL: https://codereview.chromium.org/1844363004

Cr-Commit-Position: refs/heads/master@{#385085}
(cherry picked from commit 8466be358c11657cb90fe148c10017573e6cabdc)

Review URL: https://codereview.chromium.org/1864783002 .

Cr-Commit-Position: refs/branch-heads/2661@{#497}
Cr-Branched-From: ef6f6ae5e4c96622286b563658d5cd62a6cf1197-refs/heads/master@{#378081}

[modify] https://crrev.com/b527df8f6c0fe8da64f67b58143ee34c4e6133be/ios/web/web_state/ui/crw_web_controller.mm

Labels: -Hotlist-Merge-review -Merge-Review-50
Labels: Release-0-M50
Labels: -Type-Bug Type-Bug-Security
Labels: Hotlist-WKWebViewRegression
Project Member

Comment 21 by sheriffbot@chromium.org, Jul 13 2016

Labels: -Restrict-View-SecurityNotify
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
Status: Verified (was: Fixed)
Verified on M51,52 and M53 Chrome.
Device: iPhone6, iPhone5
iOS: 9.3.2, 10.0
Project Member

Comment 23 by sheriffbot@chromium.org, Oct 1 2016

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
Project Member

Comment 24 by sheriffbot@chromium.org, Oct 2 2016

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
Labels: allpublic
Components: -Security>UX
Labels: Team-Security-UX
Security>UX component is deprecated in favor of the Team-Security-UX label

Sign in to add a comment