Interesting. 

The fix for 748362 was commit 7055d46ff097dedbb75bafc5e8880 which landed @490415 which appears to be just before the build in the screenshot.

I wonder if this is related to how the ViewHttpCacheJob surfaces notifications (it doesn't surface many)?

void ViewHttpCacheJob::OnStartCompleted() {
  NotifyHeadersComplete();
}

Fascinating. Confirmed OS X repro on 62.0.3170.0 (Official Build) canary (64-bit)	e4aa73e1d87f2a8253311b61e479589c2d7ededa-refs/heads/master@{#490562}

Interestingly, the security indicator adopts the "green" treatment (showing both the indicator and the scheme in green). In some cases, that goes away (leaving gray for both) while in others it persists.

In terms of severity, I think this is Low at worst because it requires significant user-interaction (manually entering Chrome://cache in the omnibox while on an attacker's page) and the spoof isn't terribly exciting. But it's exciting that we can get into this busted state and we may discover inconsistencies of greater impact.

elawrence@,  emilyschechter@ -- this seems like a SecurityIndicators bug, more than a WebUI bug. Do you know who would be the right person to investigate this?

It's unquestionably an issue in the security indicator, but the indicator is fed data from lower in the stack. The WebUI node is where the last bug in the ViewCache URLRequest subclass was.

I have a few ideas about why this happens. I'll take a look. 

Internally, chrome://cache is really just a virtual URL for chrome://view-http-cache [1]. 

These steps reliably demonstrate the bug:

  1. Open the test case.
  2. Visit chrome://cache on the same tab
  3. Click quickly on "Click here" link.

In contrast, these steps don't show the bug:

  1. Open the test case.
  2. Visit chrome://view-http-cache on the same tab
  3. Click quickly on "Click here" link.

This strongly suggests that the root cause of this issue is that the virtual URL isn't getting cleared off at the expected time.

[1] https://cs.chromium.org/chromium/src/chrome/browser/browser_about_handler.cc?l=74&rcl=5a3e1a066922389f0ae4892ace98c0b6ae855d96

Interesting, the omnibus always shows Chrome://cache even after navigating to another origin and back.

Deleted: Enregistrement #6.mp4 638 KB

	Enregistrement #6.mp4 638 KB View Download

I'm now able to reproduce this on Windows.

When the NavigationHandleImpl::NavigationHandleImpl is created, it attempts to match the handle to a pending navigation entry. If the pending navigation entry has a VirtualURL, then that VirtualURL is used by the new handle, leading to the spoof. 

https://cs.chromium.org/chromium/src/content/browser/frame_host/navigation_handle_impl.cc?l=156&rcl=beb884227d7b8759c461fc6ab43effb8ad479e2e

  // Try to match this with a pending NavigationEntry if possible.
  NavigationControllerImpl* nav_controller =
      static_cast<NavigationControllerImpl*>(
          frame_tree_node_->navigator()->GetController());
  if (pending_nav_entry_id_ && nav_controller) {
    NavigationEntryImpl* nav_entry = nav_controller->GetEntryWithUniqueID(pending_nav_entry_id_);
    if (!nav_entry && nav_controller->GetPendingEntry() &&
        nav_controller->GetPendingEntry()->GetUniqueID() == pending_nav_entry_id_) {
      nav_entry = nav_controller->GetPendingEntry();
    }

At this point, we have url=="https://www.yahoo.com/" with nav_entry->GetVirtualURL() == chrome://view-http-cache/, a spoof. 

Now, why is the pending_nav_entry_id_ the same between the navigations?

(Surprisingly) In the repro we see DidStartProvisionalLoad for the *second* navigation before that of the *first* navigation.

 NavigatorImpl::DidStartProvisionalLoad https://www.yahoo.com
 NavigatorImpl::DidStartMainFrameNavigation to https://www.yahoo.com 
     [At this point has_browser_initiated_pending_entry==true with a pending_entry URL of chrome://view-http-cache/]
 NavigationHandleImpl::NavigationHandleImpl for https://www.yahoo.com pending_nav_entry_id==3
 NavigatorImpl::DidStartProvisionalLoad chrome://view-http-cache/
 NavigationHandleImpl::NavigationHandleImpl for chrome://view-http-cache/ pending_nav_entry_id==3

So, perhaps the fix here is to change NavigatorImpl::DidStartMainFrameNavigation's:

  bool has_browser_initiated_pending_entry =
      pending_entry && !pending_entry->is_renderer_initiated();

to

  bool has_browser_initiated_pending_entry =
      pending_entry && !pending_entry->is_renderer_initiated() 
      && pending_entry->GetURL() == url;

?

This seems to resolve the spoof.

Doing URL comparisons for determining browser state usually has issues, so we should be careful before adding that.  Adding clamy@ and csharrison@, who have looked at this kind of matching between NavigationHandles and NavigationEntries before.  I can look more as well when I have moment.

Pinging this bug. Have we had any progress on this? Do we have a better idea for what the fix should look like (particularly re: #13)?

Are we sure this is still happening? The mechanism for the issue described in #12 relies on pre-PlzNviagte behavior.

If it still happens post-PlzNavigate, I'm in the middle of a large refactoring of NavigationController to deal with this kind of issues. First CL for the refactoring is in review here: https://chromium-review.googlesource.com/c/chromium/src/+/904988. Our goal is to eventually remove the pending NavigationEntry in favor of using the NavigationRequest itself. 

chrome://cache was removed[1] in Chrome 66, so the original POC no longer exists. I wasn't able to reproduce any badness when using the similar chrome://sync as the URL instead.

I'll mark this one as FIXED given the removal of the code. If you can reproduce this using some other Chrome URL, I'm happy to reactivate.

[1] https://chromium.googlesource.com/chromium/src/+/6ebc11f6f6d112e4cca5251d4c0203e18cd79adc 

I'm afraid the panel declined to reward for this one.

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