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

Issue 918137 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jan 9
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Bug-Regression



Sign in to add a comment

CSP is not injected from uBlock Origin when a website is opened via a tile in chrome://newtab on a FIRST VISIT

Reported by sscar...@gmail.com, Dec 28

Issue description

UserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/73.0.3635.0 Safari/537.36

Steps to reproduce the problem:
1. Have the latest bleeding edge build of Chromium or take one from https://download-chromium.appspot.com/

2. Open chrome.exe and have uBO installed or install it from https://chrome.google.com/webstore/detail/ublock-origin/cjpalhdlnbpafiamejdnhcphjbkeiagm?hl=en

3. In chrome://newtab, click on the Edit shortcut button which appears like 3 dots vertically stacked on an empty or an existing tile and fill the information like this - https://i.gyazo.com/64761dd41bd6345b82f7ad3219ca3e0d.png

4. Open chrome-extension://cjpalhdlnbpafiamejdnhcphjbkeiagm/dashboard.html and click on My Filters tab, add ||ghacks.net^$csp=default-src 'none' and click on Apply Changes and close the tab.

5. Close the browser and open it again, open chrome://newtab, click on the ghacks.net tile we created above and when the website opens it should appear broken because of the CSP filter we added in uBlock Origin in step 4, but it doesn't. The page loads resources successfully which shouldn't have happened.

What is the expected behavior?
resources should be blocked because of a CSP filter being present as mentioned in step 4.

What went wrong?
Resources are getting loaded successfully on a first visit despite of a CSP filter being present. 

WebStore page: uBlock Origin

Did this work before? Yes 73.0.3635.0 

Chrome version: 73.0.3635.0  Channel: canary
OS Version: 10.0
Flash Version: 

This is regression as this didn't happened back in 73.0.3635.0
 
Bisected to r617708 = 93e7c55ce38ad9dbed722f532c3ab76715d467d1 = crrev.com/c/1379217 by nasko@chromium.org
"Add initiator origin information to all renderer-initiated navigations."
Landed in 73.0.3645.0

Did that bisect break this ?
Does the CSP get applied later on subsequent loads or does it not work at all even after the first load?
Works fine on a refresh of the page.
Subsequent intra-site navigation within the tab is processed by uBlock.
Only the initial NTP entry click is affected.
But if I close the tab and go back to chrome://newtab, click on the ghacks tile and open it from there again, it happens again. 
Cc: rdevlin....@chromium.org nasko@chromium.org
Adding rdevlin.cronin@ as owner of extensions.
Evidently the problem is caused by these two factors:
1. the NTP page has chrome://newtab origin which is off-limits for extensions
2. webRequest API requires the 'initiator' to be accessible for an extension since Chrome 72 r609787

While the intentions were good, the reported case certainly can't be working-as-intended.
So that's a side-effect of https://crrev.com/609787 ?
It seems to me that if 2. is indeed required, then the fact that this worked was a bug, not a feature. 

What the changes in crrev.com/c/1379217 did was to add the initiator origin of navigations to all the different ways a navigation can start. Before that change, the OpenURL IPC did not carry this information, which meant that the extensions subsystem likely couldn't enforce the initiator permissions. Now the extensions code can enforce it, since it is present and likely that is what has regressed it. I haven't verified in debugger, as I don't have an easy access to one right now, but I'm almost certain that would be the root cause.

I'd leave it to rdevlin.cronin@ to chime in whether this is breaking behavior and we should be doing something about it, but from what I can see it looks like the desired behavior.

woxxom@, thank you very much for helping troubleshoot this issue and providing the extra context!
Comment #9, yes, I think this is more attributable to https://crrev.com/609787, since it changed the extension API expectation/behavior.
The status still remains Unconfirmed. Were you not able to reproduce it ?
No, I personally was not able to reproduce it, since the edit menu given in the repro steps wasn't available on the Mac and I didn't have a Windows machine handy. I've left it unconfirmed, so extensions folks can indeed confirm this is the expected behavior or tell us otherwise.
Owner: karandeepb@chromium.org
Status: Assigned (was: Unconfirmed)
Over to karandeepb@.  This is definitely interesting.  Checking for the initiator URL was intended to block extensions from intercepting requests from a page they don't have access to and indirectly modifying that page, but I don't think it should apply to top-level navigations.  Karan, WDYT?
Addendum - It doesn't happen if I right click on the website tile and click on Open link in new tab.
..also with doesn't happen with Bookmarks too.
Yes, it won't happen in those cases, as there is no document that initiated the navigation, it was started by the browser process itself - both for the context menu and the bookmarks.
>> Over to karandeepb@.  This is definitely interesting.  Checking for the initiator URL was intended to block extensions from intercepting requests from a page they don't have access to and indirectly modifying that page, but I don't think it should apply to top-level navigations.  Karan, WDYT?

Yeah this seems to have been an oversight on our part. We should not be checking the initiator for main-frame navigation requests. In fact, I am not sure if we should be checking the initiator for even sub-frame navigation requests. It seems to me that it an extension has access to a frame, it should be possible for it to block/redirect it, regardless of its parent frame's origin. Thoughts? 
Status: Started (was: Assigned)
Project Member

Comment 20 by bugdroid1@chromium.org, Jan 9

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

commit 455363f3a2cf47eb04c13bc65cb80e72387d2252
Author: Karan Bhatia <karandeepb@chromium.org>
Date: Wed Jan 09 01:19:04 2019

WebRequest: Extensions intercepting navigation requests don't need access to initiator.

This CL makes the following changes to the host permission model for the
Web Request and Declarative Net Request APIs:

- To intercept a navigation request (sub-frame or main-frame request),
  extensions don't need host permissions to the request initiator. They only
  need access to the request url. This is unlike sub-resource requests which
  need host permission to both the request url and initiator.
- If an extension has access to a navigation request's initiator but it's access
  to the request url is withheld, the extension isn't implicitly granted access
  to the request. This is unlike sub-resource requests where the extension will
  be granted access.

BUG=157736,  918137 , 851722

Change-Id: I851bfbe79ea737b59934779367d581cdbb6c5297
Reviewed-on: https://chromium-review.googlesource.com/c/1396564
Commit-Queue: Karan Bhatia <karandeepb@chromium.org>
Reviewed-by: Devlin <rdevlin.cronin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#620977}
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/declarative_net_request/declarative_net_request_browsertest.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/web_request/web_request_apitest.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/browser/extensions/api/web_request/web_request_permissions_unittest.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/common/extensions/docs/templates/intros/declarativeNetRequest.html
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/chrome/common/extensions/docs/templates/intros/webRequest.html
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/declarative_net_request/ruleset_manager.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/declarative_webrequest/webrequest_action.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_api.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_event_details.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions.cc
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions.h
[modify] https://crrev.com/455363f3a2cf47eb04c13bc65cb80e72387d2252/extensions/browser/api/web_request/web_request_permissions_unittest.cc

Status: Fixed (was: Started)
This should be fixed by c#20. I don't think this requires a merge to M72 since this only became an issue in combination with crrev.com/c/1379217 (m73), which added initiator info. to renderer initiated navigations to remote frames and those corresponding to window.open. Before that only local/same-process renderer initiated navigations had initiator info.
I want to test, so where did the fix landed version wise ?
It landed at 73.0.3666.0. It should be available on the latest Mac, Win dev, canary.
Thank you. It works as expected in 73.0.3666.0.

Sign in to add a comment