Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ghostery for Chrome unexpectedly blocks content with class="clickable" #132

Closed
aaronadamsCA opened this issue Jul 12, 2018 · 18 comments
Closed

Comments

@aaronadamsCA
Copy link

@aaronadamsCA aaronadamsCA commented Jul 12, 2018

Description

When I open a local file via file: protocol, and that file includes an iframe via https: protocol, Ghostery unexpectedly inserts a rule into the iframe that suppresses some content - specifically, any element with class "clickable".

Expected Behavior

Nothing. Ghostery says it only scans http and https pages; I wouldn't expect it to do anything in this case, let alone this.

Actual Behavior

Ghostery appears to insert <style type="text/css" id="cliqz-adblokcer-css-rules"> :root .clickable {display:none !important;}</style> to the <head> element of the iframe, causing some of its content to be suppressed.

Steps to Reproduce

  1. Open this Gist: https://htmlpreview.github.io/?https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0/raw/bug.html
  2. Save bug.html locally
  3. Open bug.html in your browser via the file: protocol, while running Ghostery for Chrome 8.2.0

Note that paragraph 2 inside the iframe is no longer visible. This appears to be due to Ghostery unexpectedly inserting a rule into the iframe page's <head> element.

This appears to happen only when the parent page is accessed via file:, and the child page via https: (or http:). No other combination, including visiting the child page directly, appears to cause Ghostery to insert this rule.

(I know it seems super unlikely that anyone would run into this in the wild - but I did, via work on a platform that involves third-party JavaScript and iframes. It was super confusing until I figured out Ghostery was messing with me, likely inadvertently.)

Versions

  • Browser: Chrome 68
  • OS: Windows 10
  • Extension: Ghostery for Chrome 8.2.0
@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Jul 12, 2018

Here's the Gist page: https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0

I didn't realize I could leave some screenshots too, here you go! Proof that I might not be crazy.

1-https-parent-noissue
2-local-parent-issue
3-https-child-only-noissue
4-local-child-only-noissue

@Aziz-Ghostery
Copy link

@Aziz-Ghostery Aziz-Ghostery commented Jul 16, 2018

I'm unable to reproduce this issue, please see attached screenshot.
bug html not scanned

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Jul 17, 2018

Try enabling Enhanced Ad Blocking - it appears to be associated with that feature. When I turn it off, the issue disappears.

I tried to debug a little further, too. First I put a breakpoint on (I think) this line:

https://github.com/cliqz-oss/adblocker/blob/dcf57a26e431d8cd1427c0eddec3cfbfc3a23a96/src/cosmetics-injection.ts#L172

That got me a stack trace:

ghostery-stack-trace

It looks like this was all kicked off by a background message that looked like this:

{
  "response": {
    "active": true,
    "blockedScripts": [],
    "scripts": [],
    "styles": [
      ".clickable",
      "#oas-mpu-left\\<\\/div\\>",
      "#oas-mpu-right\\<\\/div\\>",
      "a[href^=\"http://redir.widdit.com/redir/?\"] > *"
    ]
  }
}

I can't figure out where that message came from; that's beyond my debugging capabilities. But if I had to guess - some kind of a bug in the underlying rules engine, causing Ghostery to receive rules that shouldn't actually apply? Or maybe even just some old dead code that's unexpectedly come back to life?

@philipp-classen
Copy link
Contributor

@philipp-classen philipp-classen commented Aug 10, 2018

Hi Aaron, thanks for your help. Can you still reproduce it with the latest release (8.2.3)?

I can see that there were some changes in the injection logic recently.

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Aug 10, 2018

Unfortunately I'm still able to reproduce in version 8.2.3, under these (admittedly very specific!) circumstances.

@philipp-classen
Copy link
Contributor

@philipp-classen philipp-classen commented Aug 10, 2018

Weird. I could reproduce, although I don't understand the pattern:

  • In a existing profile with Chrome 68 and Ghostery 8.2.3, I saw both. With a fresh profile, I saw only one.
  • In the Cliqz Browser with an existing profile and Ghostery 8.2.1, I saw one. But with a fresh profile, I saw two.

(Tested on Linux. But I don't think it is OS related.)

Maybe it is only triggered by some race. But afterwards it was consistent even when clearing caches and reloading.

I can check next week if it is also reproducible with the browser-core master.

@philipp-classen
Copy link
Contributor

@philipp-classen philipp-classen commented Aug 17, 2018

There is a adblocker rule that hide the clickable element in the frame. I don't think it is a bug here, but instead intended behavior to prevent clickjacking attacks.

If you follow the network request ...

GET https://htmlpreview.github.io/?https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0/raw/iframe.html

->

GET https://htmlpreview.github.io/htmlpreview.min.js

->

GET https://query.yahooapis.com/v1/public/yql?q=use "https://raw.githubusercontent.com/yql/yql-tables/master/data/data.headers.xml" as headers; select * from headers where url="https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0/raw/iframe.html"&format=json&diagnostics=true&callback=HTMLPreview.loadHTML

-->

/**/HTMLPreview.loadHTML({"query":{"count":1,"created":"2018-08-17T13:28:23Z","lang":"en-US","diagnostics":{"url":[{"execution-start-time":"1","execution-stop-time":"1","execution-time":"0","content":"https://raw.githubusercontent.com/yql/yql-tables/master/data/data.headers.xml"},{"execution-start-time":"4","execution-stop-time":"168","execution-time":"164","content":"https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0/raw/iframe.html"}],"publiclyCallable":"true","log":"response length: 238","javascript":{"execution-start-time":"2","execution-stop-time":"168","execution-time":"166","instructions-used":"15235","table-name":"headers"},"user-time":"169","service-time":"164","build-version":"2.0.301"},"results":{"resources":{"url":"https://gist.github.com/aaronadamsTO/f9206dc5e2e3f2217bbde8fcb92609b0/raw/iframe.html","status":"200","headers":{"result":{"content-security-policy":"default-src 'none'; style-src 'unsafe-inline'; sandbox","strict-transport-security":"max-age=31536000","x-content-type-options":"nosniff","x-frame-options":"deny","x-xss-protection":"1; mode=block","etag":"\"6e0eeb240557ec7aa59617f15451f59037403152\"","content-type":"text/plain; charset=utf-8","cache-control":"max-age=300","x-geo-block-list":null,"x-github-request-id":"8DE2:27E9:38D6:3BF9:5B76CCF2","content-length":"237","accept-ranges":"bytes","via":"1.1 varnish, https/1.1 ecq3.ycs.bf1.yahoo.com (ApacheTrafficServer [cMsSfW])","x-served-by":"cache-ewr18125-EWR","x-cache":"HIT","x-cache-hits":"1","x-timer":"S1534512503.113125,VS0,VE0","vary":"Authorization,Accept-Encoding","access-control-allow-origin":"*","x-fastly-request-id":"c63137ea6782aa0e76b307b548ac5a95a4471798","expires":"Fri, 17 Aug 2018 13:33:23 GMT","source-age":"129","date":"Fri, 17 Aug 2018 13:28:23 GMT","age":"0","connection":"keep-alive","server":"ATS"}},"content":"<!DOCTYPE html>\n<html>\n<head>\n    <title>iframe.html - Ghostery extension bug</title>\n</head>\n<body>\n    <p>This paragraph is always visible</p>\n    <p class=\"clickable\">The Ghostery extension will hide this paragraph</p>\n</body>\n</html>"}}}});

... you will also see that Github sets the "x-frame-options":"deny" header, which also is intended to prevent clickjacking.

https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/X-Frame-Options

In sum, the rule to hide clickable elements in the iframe makes sense in my understanding.

@philipp-classen
Copy link
Contributor

@philipp-classen philipp-classen commented Aug 17, 2018

Forgot to mention. The last test has with the browser-core master version. I also did not notice erratic behavior there but it was more consistent:

When Ghostery was paused (or in Cliqz when the adblocker was disabled), it was shown, otherwise it was hidden.

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Aug 23, 2018

I only used GitHub because it was the easiest way to share the example with its source. And the iframed page does not set X-Frame-Options and is intended to be permitted to be iframed.

It's also worth noting the rule absolutely does not block clickable elements inside iframes. It only blocks elements carrying an explicit class="clickable".

It really seems more like a mistake with the rules engine passing a rule that was intended for some other site to localhost sites instead.

If there's a better site on which to create the example that would avoid the red herring of the HTML preview JS loader, I can create a sample there. Maybe that would make this a simple investigation instead of a complicated one?

@remusao
Copy link
Contributor

@remusao remusao commented Sep 25, 2018

Hi @aaronadamsTO,

Do you still see this issue? I was checking the rules and it seems that the only one which could be problematic is this one: telegraphindia.com##.clickable from easylist, but it should only be used on the telegraphindia.com domain. I could not reproduce the issue yet though.

@butlerfr
Copy link

@butlerfr butlerfr commented Nov 9, 2018

Same issue for me but in Firefox 63.0.1 (64bits).
I have a file including multiples iframe for monitoring some services.

If I turn off ghostery everything work fine but if I turn it back on i have just blank frame...

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Nov 9, 2018

I can confirm the issue has worsened significantly. Ghostery now inserts these rules into all iframes on HTML pages opened on file: protocol:

<style type="text/css" id="cliqz-adblokcer-css-rules"> :root [style]:not(SPAN) {display:none !important;}</style>
<style type="text/css" id="cliqz-adblokcer-css-rules"> :root .header {display:none !important;}</style>

I'm thinking these could be malformed rules breaking/confusing the rules engine.

Keep in mind that on file: protocol, document.domain = "". Could it be possible that malformed rules currently flow to blank domains instead of real ones?

@remusao
Copy link
Contributor

@remusao remusao commented Nov 9, 2018

Thank you for the details about the issue @butlerfr @aaronadamsTO. Indeed, it looks like empty domain might be the root cause of the issue. I suspect that an empty domain might allow triggering of rules meant for specific domains. I will look into this issue ASAP.

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Nov 9, 2018

Thanks @remusao.

If it helps, I'm guessing the rule that's now suppressing iframes entirely is probably this one, checked in 12 days ago:

https://github.com/cliqz-oss/adblocker/blob/0cddd63ec5cf46d71f2d39b8e8887172b2ab6feb/assets/raw.githubusercontent.com/uBlockOrigin/uAssets/master/filters/filters.txt#L10997

rmdown.com##[style]:not(SPAN)
@remusao
Copy link
Contributor

@remusao remusao commented Nov 9, 2018

@aaronadamsTO I was able to reproduce and find a fix for this issue, which was two-fold:

  1. We should not inject cosmetics at all on un-supported protocols (e.g. file://), as you suggested initially. There was one code-path which did not check this condition and would potentially inject cosmetics for URLs with any protocol.
  2. In some cases, when hostname is empty, some rules would be incorrectly matched. Again, as pointed out by you, this could happen with file:/// protocol.

The fixes will be pushed to ghostery-extension soon. I will keep you updated on the status.

Thanks again for investigating, this was really helpful.

@christophertino
Copy link
Member

@christophertino christophertino commented Nov 27, 2018

@aaronadamsTO this should be resolved in Ghostery v8.2.5. Please upgrade and let us know if it's still happening.

@aaronadamsCA
Copy link
Author

@aaronadamsCA aaronadamsCA commented Nov 27, 2018

@christophertino Success! I'm happy to confirm Ghostery is no longer injecting strange rules into my iframes in domainless contexts. My test pages work again!

This will save me multiple "WTF? Guh..." trips to Incognito mode every week, so thank you!

@christophertino
Copy link
Member

@christophertino christophertino commented Nov 27, 2018

Great! Thanks again for the help, much appreciated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
7 participants