Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

chromium code search won't display with fingerprinting protection #12143

Closed
darkdh opened this issue Nov 30, 2017 · 10 comments
Closed

chromium code search won't display with fingerprinting protection #12143

darkdh opened this issue Nov 30, 2017 · 10 comments

Comments

@darkdh
Copy link
Member

darkdh commented Nov 30, 2017

Description

As title, this happens since 0.19.99

Steps to Reproduce

  1. Go to https://cs.chromium.org
  2. Turn on fingerprinting protection per site or globally
  3. The page is white and the fingerprint protection count is 0

Actual result:

screen shot 2017-11-30 at 8 02 35 am

Expected result:
It should be viewable with fingerprinting protection(It used to be)

Reproduces how often:

Brave Version

about:brave info:

Brave: 0.19.100
rev: 55a964a
Muon: 4.5.16
libchromiumcontent: 62.0.3202.94
V8: 6.2.414.42
Node.js: 7.9.0
Update Channel: Release
OS Platform: macOS
OS Release: 17.2.0
OS Architecture: x64

Reproducible on current live release:

No

Additional Information

cc @diracdeltas @bbondy

@darkdh
Copy link
Member Author

darkdh commented Nov 30, 2017

cc @snyderp

@bsclifton
Copy link
Member

Possibly caused by #11784

@bsclifton bsclifton modified the milestones: 0.19.x Hotfix 6 (Release channel), 0.19.x + C63 (Release Channel) Nov 30, 2017
@darkdh
Copy link
Member Author

darkdh commented Nov 30, 2017

same thing happens on https://productforums.google.com/forum/

@pes10k
Copy link
Contributor

pes10k commented Nov 30, 2017

Yep, this is definitely caused by #11784

Lines of code like this (ugly b/c its from be debugging the minified source)

function OCd(b){try{if(!b.contentWindow||!b.contentWindow.document)return null;return b.contentWindow.document.body.innerHTML}

I'm not sure what is best here. I don't think there is any JS/extension/short term solution for #11784 that won't also cause this pattern to break.

Barring some other quick solution for #11784 that I can't think of, I think the options are:

  1. Say "shields will break some sites", and these are examples of them (not totally satisfying…)
  2. Some kind of allow list to allow well known domains to talk cross frames
  3. Roll back block access to child frames' contentWindow, contentDocument, place guard in blocking proxy to prevent an infinite loop #11784 and accept that Brave's fingerprinting protections can be circumvented (at least until fingerprinting protection should be done in C++ instead of js content script #12045)
  4. Another option would be to hack electron to stop parent frames until the document_start hook of the child frame fires (which would solve block access to child frames' contentWindow, contentDocument, place guard in blocking proxy to prevent an infinite loop #11784 Fingerprinting protections bypassable via window.open #12110 and similar)

All those are different degrees of unsatisfying. I'm sure I'm not the person to evaluate the tradeoffs between them, though I'd be very happy to help push forward with any of those that you all decide to go with. @diracdeltas ?

@diracdeltas
Copy link
Member

diracdeltas commented Nov 30, 2017

i think the best option is 3, roll back #11784, unless 4 can be done really quickly

@pes10k
Copy link
Contributor

pes10k commented Nov 30, 2017

Realistically, I think 4 would take less time than #12045 (though couldn't swear to it) but it definitively wouldn't be small thing. Days or a week, plus testing.

I'll start looking into #12045 more seriously then.

Is there a test suite for these kinds of issues? Even if they're live sites and the tests might have to be updated periodically to account for changes, it might be worth the lift starting up

@bsclifton bsclifton modified the milestones: 0.19.x + C63 (Release Channel), 0.19.x Hotfix 6 (Release channel) Nov 30, 2017
@bsclifton
Copy link
Member

Fixed with fe2fced

@kjozwiak kjozwiak added 0.19.x issue first seen in 0.19.x bug labels Dec 1, 2017
@kjozwiak
Copy link
Member

kjozwiak commented Dec 1, 2017

@darkdh has fe2fced fixed the issues you were having? QA will also check and ensure that the above two websites are now working on the three main platforms.

@kjozwiak
Copy link
Member

kjozwiak commented Dec 1, 2017

Test Case Used:

@darkdh
Copy link
Member Author

darkdh commented Dec 1, 2017

Verified it works

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants