search

ghostery / ghostery-extension

Ghostery Browser Extension for Firefox, Chrome, Opera, Edge and Safari
Mozilla Public License 2.0
1.37k stars 139 forks source link

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

Closed aaronadamsCA closed 5 years ago

aaronadamsCA commented 6 years ago

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

aaronadamsCA commented 6 years ago

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 commented 6 years ago

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

aaronadamsCA commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

philipp-classen commented 6 years ago

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

(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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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

aaronadamsCA commented 5 years ago

@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 commented 5 years ago

Great! Thanks again for the help, much appreciated.