Closed Bug 1190450 Opened 4 years ago Closed 4 years ago

Tracking Protection fires onSecurityChange with the wrong webProgress.currentURI when the tracking request happens onunload

Categories

(Core :: Security, defect, P1)

defect

Tracking

()

VERIFIED FIXED
mozilla43
Iteration:
43.1 - Aug 24
Tracking Status
firefox40 --- wontfix
firefox41 --- wontfix
firefox42 + verified
firefox43 --- verified

People

(Reporter: bgrins, Assigned: valentin.gosu)

References

(Depends on 1 open bug)

Details

(Whiteboard: [fxprivacy])

Attachments

(1 file)

This is a follow up to the investigation in Bug 1185117, specifically Comments 5 and 6.

STR:

Open a new tab to page 1: data:text/html,hi
Enter in this URL to go to page 2: https://bug1185117.bmoattachments.org/attachment.cgi?id=8642472
Press back

Expected:

The tracking shield does not show up on Page 1 after going back

Actual:

The tracking shield shows up on Page 1 after going back

The reason the shield shows up is because there is an onSecurityChange notification that fires with STATE_BLOCKED_TRACKING_CONTENT.  And if you log out the webProgressListener.currentURI or webProgressListener.DOMWindow.location, it's returning Page 1's URI even though the tracking request originated from Page 2.
I'd expect either:

1) no STATE_BLOCKED_TRACKING_CONTENT notification should fire if the page has already navigated away
2) the currentURI and other security info should be set correctly so that consumers can distinguish between this and a legitimate onSecurityChange call from the first page
Flags: qe-verify?
Flags: firefox-backlog+
Priority: -- → P2
Whiteboard: [fxprivacy]
Flags: qe-verify? → qe-verify+
Blocks: 1188565
Assignee: nobody → ttaubert
Status: NEW → ASSIGNED
Iteration: --- → 43.1 - Aug 24
Priority: P2 → P1
Brian, I can't reproduce this. trackertest.org doesn't seem to exist, is that why the attachment fails to help reproduce this?
Flags: needinfo?(bgrinstead)
I can reproduce with the original STR from bug 1185117 though.
Assignee: ttaubert → valentin.gosu
[Tracking Requested - why for this release]:

As we're planning to ship TP in 42 we should definitely uplift the suggested fix to prevent false positives.
Flags: needinfo?(bgrinstead)
Attachment #8646902 - Flags: review?(mcmanus) → review+
QA Contact: mwobensmith
https://hg.mozilla.org/mozilla-central/rev/827df7f198cf
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
Tracking to make sure we don't miss it.
Valentin, could you fill the uplift request to 42? Thanks
Flags: needinfo?(valentin.gosu)
Comment on attachment 8646902 [details] [diff] [review]
TP shield shows up incorrectly when pressing back while loading a page with tracking elements

Approval Request Comment
[Feature/regressing bug #]:
Always existed.

[User impact if declined]:
TP shield will appear on pages with no tracking content if you click back before a page has finished loading.

[Describe test coverage new/current, TreeHerder]:
None. Tested manually.

[Risks and why]: 
Low risk. This adds a check that the current window, to which we add the TP shield, is the same as the one that opened the channel containing tracking content.

[String/UUID change made/needed]:
None.
Flags: needinfo?(valentin.gosu)
Attachment #8646902 - Flags: approval-mozilla-aurora?
Tim, you mentioned on IRC that it would be easy to add an automated test for this. If you don't have time to do it yourself, could you provide some info about how it would work? Thanks!
Flags: needinfo?(ttaubert)
Comment on attachment 8646902 [details] [diff] [review]
TP shield shows up incorrectly when pressing back while loading a page with tracking elements

TP is a new feature, taking it to polish it.
Attachment #8646902 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(In reply to Valentin Gosu [:valentin] from comment #10)
> Tim, you mentioned on IRC that it would be easy to add an automated test for
> this. If you don't have time to do it yourself, could you provide some info
> about how it would work? Thanks!

Hmm, iirc the issue wasn't easy to reproduce reliably, but attachment 8642472 [details] might have a way to do that. It just needs to try loading a tracker which didn't work at the time I tried, for some reason. It might be enough to just follow comment #0, load a tab, navigate to the test page, and navigate back. Then check whether the security state indicates a tracker is present.
Flags: needinfo?(ttaubert)
Reproduced couple of times on Nightly 2015-08-03.
Verified fixed 42.0a2 (2015-09-10), 43.0a1 (2015-09-10) Win 7
Status: RESOLVED → VERIFIED
Depends on: 1451307
You need to log in before you can comment on or make changes to this bug.