Closed Bug 1485421 Opened 8 years ago Closed 2 years ago

Storage access is granted when a user interacts with embedded documents that have an opener relationship

Categories

(Core :: Privacy: Anti-Tracking, enhancement, P3)

enhancement

Tracking

()

RESOLVED WONTFIX

People

(Reporter: englehardt, Assigned: timhuang)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

In Bug 1469993 we implemented a storage access grant under the tracking cookie restrictions policy when a user interacts with an opened document. This interaction heuristic is meant to complement the window.open heuristic implemented in Bug 1474651, such that Bug 1474651 grants access to the initial origin passed to window.open and Bug 1469993 grants access to the final origin which the user interacts with. The interaction heuristic currently allows any document that has an opener relationship to the original page to activate storage access, regardless of how or where that document is opened. Here are two examples that we should not allow: 1. The tracker page is opened through the following link: <a href="tracker.example" target="_blank">Click here</a>. If the user interacts with the resulting window, storage access is granted. 2. On example.com, the tracking script creates an empty iframe with a name attribute. Then, the tracking script calls window.open("tracker.example", "iframename"), which opens an iframe in the page. If the user interacts with that iframe, storage access for `tracker.example` is granted on example.com. For consistency, it seems we should only grant storage access for documents that were the result of a window.open() that would have granted storage access to origin passed to the initial call. Alternatively, we could consider granting storage access to the initial origin set as the href attribute on anchor tags that create new windows (i.e, <a href="" target="_blank">), continue to permit activation in example (1) above, and just check that the document is top-level to exclude example (2).
(In reply to Steven Englehardt [:englehardt] from comment #0) > In Bug 1469993 we implemented a storage access grant under the tracking > cookie restrictions policy when a user interacts with an opened document. > This interaction heuristic is meant to complement the window.open heuristic > implemented in Bug 1474651, such that Bug 1474651 grants access to the > initial origin passed to window.open and Bug 1469993 grants access to the > final origin which the user interacts with. > > The interaction heuristic currently allows any document that has an opener > relationship to the original page to activate storage access, regardless of > how or where that document is opened. Here are two examples that we should > not allow: > > 1. The tracker page is opened through the following link: <a > href="tracker.example" target="_blank">Click here</a>. If the user interacts > with the resulting window, storage access is granted. Why is that? Do you have a test case that shows that Safari treats such pages differently than those opened by window.open()? Since the platform doesn't really differentiate between these pages in any real way, besides not giving the opener a reference to the opened window, I find it strange to special case one set of windows here but not others. Last I checked, webkit uses the existence of the opener reference for their heuristic. All else being equal, I prefer to not diverge from webkit's heuristic unless if web compatibility forces us to. > 2. On example.com, the tracking script creates an empty iframe with a name > attribute. Then, the tracking script calls window.open("tracker.example", > "iframename"), which opens an iframe in the page. If the user interacts with > that iframe, storage access for `tracker.example` is granted on example.com. > > For consistency, it seems we should only grant storage access for documents > that were the result of a window.open() that would have granted storage > access to origin passed to the initial call. Do you have a test case that shows that we have the behavior you're describing here? The code we have has certainly tried to prevent the scenario (2) from happening (see how |!windowExists| is being checked here: <https://searchfox.org/mozilla-central/rev/3fa761ade83ed0b8ab463acb057c2cf0b104689e/dom/base/nsGlobalWindowOuter.cpp#7007>) so I'd like to see a test case that shows this bug actually exists first, I'm a bit surprised... > Alternatively, we could consider granting storage access to the initial > origin set as the href attribute on anchor tags that create new windows > (i.e, <a href="" target="_blank">), continue to permit activation in example > (1) above, and just check that the document is top-level to exclude example > (2). That's definitely the wrong thing to do, because the href attribute doesn't really mean anything (due to HTTP redirects). We need to determine any action we want to take here based on the document channel's result principal (which represents the location the document is actually loaded from, not the source href, accounting for redirects etc.) That _is_ what we're doing today, FWIW.
Flags: needinfo?(senglehardt)
Priority: -- → P3
(In reply to :Ehsan Akhgari from comment #1) > (In reply to Steven Englehardt [:englehardt] from comment #0) > > In Bug 1469993 we implemented a storage access grant under the tracking > > cookie restrictions policy when a user interacts with an opened document. > > This interaction heuristic is meant to complement the window.open heuristic > > implemented in Bug 1474651, such that Bug 1474651 grants access to the > > initial origin passed to window.open and Bug 1469993 grants access to the > > final origin which the user interacts with. > > > > The interaction heuristic currently allows any document that has an opener > > relationship to the original page to activate storage access, regardless of > > how or where that document is opened. Here are two examples that we should > > not allow: > > > > 1. The tracker page is opened through the following link: <a > > href="tracker.example" target="_blank">Click here</a>. If the user interacts > > with the resulting window, storage access is granted. > > Why is that? Do you have a test case that shows that Safari treats such > pages differently than those opened by window.open()? Since the platform > doesn't really differentiate between these pages in any real way, besides > not giving the opener a reference to the opened window, I find it strange to > special case one set of windows here but not others. > > Last I checked, webkit uses the existence of the opener reference for their > heuristic. All else being equal, I prefer to not diverge from webkit's > heuristic unless if web compatibility forces us to. You're right. Safari handles case (1) the same as I've described here. Chatting offline it sounds like this is the intended functionality, so let's ignore that case. (2) is still a concern. > > 2. On example.com, the tracking script creates an empty iframe with a name > > attribute. Then, the tracking script calls window.open("tracker.example", > > "iframename"), which opens an iframe in the page. If the user interacts with > > that iframe, storage access for `tracker.example` is granted on example.com. > > > > For consistency, it seems we should only grant storage access for documents > > that were the result of a window.open() that would have granted storage > > access to origin passed to the initial call. > > Do you have a test case that shows that we have the behavior you're > describing here? The code we have has certainly tried to prevent the > scenario (2) from happening (see how |!windowExists| is being checked here: > <https://searchfox.org/mozilla-central/rev/ > 3fa761ade83ed0b8ab463acb057c2cf0b104689e/dom/base/nsGlobalWindowOuter. > cpp#7007>) so I'd like to see a test case that shows this bug actually > exists first, I'm a bit surprised... I do. You can see an example of (2) using this test page: https://github.com/englehardt/englehardt.github.io/blob/929007135a1c19f8be30e0b36be80e3832b4dab9/test/cookie_restrictions/master.html#L67-L79. Unfortunately to use it to test you'll need to clone the repo and serve the files locally as described in the body of that page. > > Alternatively, we could consider granting storage access to the initial > > origin set as the href attribute on anchor tags that create new windows > > (i.e, <a href="" target="_blank">), continue to permit activation in example > > (1) above, and just check that the document is top-level to exclude example > > (2). > > That's definitely the wrong thing to do, because the href attribute doesn't > really mean anything (due to HTTP redirects). We need to determine any > action we want to take here based on the document channel's result principal > (which represents the location the document is actually loaded from, not the > source href, accounting for redirects etc.) > > That _is_ what we're doing today, FWIW. Note that in https://searchfox.org/mozilla-central/rev/3fa761ade83ed0b8ab463acb057c2cf0b104689e/dom/base/nsGlobalWindowOuter.cpp#7007 `aCalledNoScript` will be `true` for windows created by anchor tags, so access won't be granted. As mentioned above, we've decided this is the intended functionality.
Flags: needinfo?(senglehardt)
Priority: P3 → --
Summary: Storage access is granted when a user interacts with any document that has an opener relationship → Storage access is granted when a user interacts with embedded documents that have an opener relationship
Priority: -- → P3
Attached patch 1485421.patch — — Splinter Review
This is a sample patch that should fix the issue. Right now MaybeAllowStorageForOpener is only called by the user interaction heuristic. I suspect we'd always want to check if the document is top-level, but if there are scenarios where we'd want to grant storage access for third-party tracking windows that aren't top-level, we can move the check elsewhere.
(In reply to Steven Englehardt [:englehardt] from comment #2) > > > 2. On example.com, the tracking script creates an empty iframe with a name > > > attribute. Then, the tracking script calls window.open("tracker.example", > > > "iframename"), which opens an iframe in the page. If the user interacts with > > > that iframe, storage access for `tracker.example` is granted on example.com. > > > > > > For consistency, it seems we should only grant storage access for documents > > > that were the result of a window.open() that would have granted storage > > > access to origin passed to the initial call. > > > > Do you have a test case that shows that we have the behavior you're > > describing here? The code we have has certainly tried to prevent the > > scenario (2) from happening (see how |!windowExists| is being checked here: > > <https://searchfox.org/mozilla-central/rev/ > > 3fa761ade83ed0b8ab463acb057c2cf0b104689e/dom/base/nsGlobalWindowOuter. > > cpp#7007>) so I'd like to see a test case that shows this bug actually > > exists first, I'm a bit surprised... > > I do. You can see an example of (2) using this test page: > https://github.com/englehardt/englehardt.github.io/blob/ > 929007135a1c19f8be30e0b36be80e3832b4dab9/test/cookie_restrictions/master. > html#L67-L79. Unfortunately to use it to test you'll need to clone the repo > and serve the files locally as described in the body of that page. I see! The bug happens with our user interaction heuristic, but here I was talking about our window.open() heuristic... Nice find.
One thing to note is this loop: <https://searchfox.org/mozilla-central/rev/f2ac80ab7dbde5400a3400d463e07331194dec94/dom/events/EventStateManager.cpp#508>. If the user *first* interacts with an iframe, we mark all of the ancestor documents as interacted with as well, so eventually the top-level window is going to be marked as interacted with as well. So this bug is more about making sure that we're computing the right permission, only for the top-level window...
Component: Tracking Protection → Privacy: Anti-Tracking
Product: Firefox → Core

Steven, are you still working on this? This bug seems to have fell through the cracks. :-(

Flags: needinfo?(senglehardt)

I am not still working on this. Are you able to?

Flags: needinfo?(senglehardt)

NI to Ethan to assign team member to support.

Flags: needinfo?(ettseng)

Hi Thomas, I believe you have the knowledge to resolve this bug. Let me know if you have any concern (we can chat offline).

Assignee: nobody → tnguyen
Flags: needinfo?(ettseng)

Gary, please help to look into this issue, thanks.

Assignee: tnguyen → xeonchen

Gary, please consult Englehardt or Thomas if you have any question about this bug.

Assignee: xeonchen → tihuang
Severity: normal → S3

We can close this bug because the storage access heuristics have been changed since this bug was filed. For the two cases mentioned in comment 0, we don't actually grant storage access anymore.

Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: