#18741: OCSP and favicon isolation is only partly working in ESR 45
-------------------------------------------------+-------------------------
Reporter: gk | Owner: tbb-
Type: defect | team
Priority: High | Status:
Component: Applications/Tor Browser | needs_review
Severity: Major | Milestone:
Keywords: ff45-esr, tbb-6.0a5, | Version:
TorBrowserTeam201604R | Resolution:
Parent ID: | Actual Points:
Reviewer: | Points:
| Sponsor:
-------------------------------------------------+-------------------------
Comment (by gk):
Replying to [comment:10 arthuredelstein]:
> Replying to [comment:9 mcs]:
>
> Thanks for the review. Here's a new version with 4 patches:
https://github.com/arthuredelstein/tor-browser/commits/16326+4.
> Hash: 164431b40788b18b28502804224b54cc5760083b
>
> > Can you explain why the above patch is needed? Why aren't we passing
the correct aNode in all cases? I am worried that we will poke around in
the ancestor elements looking for a "firstparty" attribute in a lot more
cases now, and I am not sure what the implications are (but I have not run
the code yet).
>
> I agree that seems dangerous. So now I'm proposing a change to the
`ThirdPartyUtil.cpp` code so that it only looks at the immediate node, and
does not traverse the ancestors.
> Hash: 3553684ab8fa75ac55b930916b7ee06c862c644e
>
> To compensate for this change, I used a XUL attribute inheritance trick
that allows the "firstparty" attribute on the `tab` element to propagate
down to its icon `xul:image`:
> Hash: e4854b02006d5b156c8e40d482b869b904eb1034
>
> These changes together give the same behavior of favicons, but hopefully
this arrangement is much less likely to affect other objects in the DOM.
Sounds like a good approach we could test in the alpha. One nit:
There is no need for an additional `node->IsElement()` check as you are
landing in the `if`-block only after the `node->IsElement()` check passes.
Otherwise looks good to me. (I assume you made sure the patch builds for
all platforms and fixes the issue and the tests are passing; I currently
don't have the means to verify that)
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/18741#comment:11>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs
