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

Allow to trust built-in third-party filters #4386

Open
contribucious opened this issue Aug 19, 2022 · 3 comments
Open

Allow to trust built-in third-party filters #4386

contribucious opened this issue Aug 19, 2022 · 3 comments

Comments

@contribucious
Copy link

contribucious commented Aug 19, 2022

Intro

Hi. πŸ‘‹ Let's take a concrete example. For reasons I'll explain in the Context section β€” sorry in advance for the length β€”, I wanted to switch from the built-in third-party Fanboy's Annoyances filter (with its trustLevel set to "low") to the original Fanboy's Annoyance List (not having this limitation). Impossible unfortunately without a workaround (i.e. to use another URL) in AdGuard for Windows β€” unlike in AdGuard Browser extension.

Proposed solutions

  • πŸ…°οΈ Either allow to check Trusted filter for built-in third-party filters …
    Possibly when it's added from the built-in list (in the add wizard) and/or … at least leave this possibility later (instead of this, have that).
Β Β Β Β View the implications …
  • The reported issues by the users should automatically mention this state for these filters.

  • The AdGuard Filters team will need to ensure that they use these same conditions for their reproducibility tests.

  • πŸ’‘ If AdGuard does not want to take the risk of hosting third-party filter versions with risky rules inside, simply use the original source when the user checks "trusted filter" on an otherwise "low" one (AdGuard knows how to handle +js rules and stuff like that anyway).

  • πŸ…±οΈ And/Or, at the very least I want to say, if a person removes the built-in third-party Fanboy's Annoyances filter and adds manually the original one via its official address (https://secure.fanboy.co.nz/fanboy-annoyance_ubo.txt) and checks Trusted filter, do not redirect/hijack the URL towards the built-in limited version β€” as it seems to be now β€” nor convert it to this limited version format at this time either (especially without notifying the user of this, I would add ☺️). AdGuard Browser extension does not suffer from this problem it seems.

πŸ” About this hijack

βœ–οΈ AdGuard for Windows current problematic state:

View it …
  • Initial state β€” Built-in third-party Fanboy's Annoyances filter added:
    screenshot
    β†ͺ️ Observe: 57551 rules.

  • When removing it then adding manually a new filter with its official URL (which is: https://secure.fanboy.co.nz/fanboy-annoyance_ubo.txt):
    screenshot
    β†ͺ️ Observe: the name is not Fanboy's Annoyance List but has become Fanboy's Annoyances with v0.0.0.0. #hijack

  • Once added:
    screenshot
    β†ͺ️ Observe: 57551 rules only, name of the AG version and auto put in "Annoyances" section.

  • NOW, when adding manually a new filter with another URL to the same location as a workaround (using the following URL instead: https://www.fanboy.co.nz/fanboy-annoyance_ubo.txt):
    screenshot
    β†ͺ️ Observe: the name is correctly Fanboy's Annoyance List with the current version of the moment.

  • Once added:
    screenshot
    β†ͺ️ Observe: 59016 rules as expected, original name kept and put in the top section ("User" section).
    Β 

βœ”οΈ AdGuard Browser extension current okay state:

View it …
  • Adding manually a new filter with its official URL (which is: https://secure.fanboy.co.nz/fanboy-annoyance_ubo.txt):
    screenshot
    β†ͺ️ Observe: 59015 rules as expected*, original name kept. NOT hijacked in this case unlike AdGuard for Windows.
    Β 
    * (So, all the 59000+ rules. No need to worry about the 1 rule of difference versus AdGuard for Windows here, the sha256sum returned value for this file both accessed via "www." and "secure." is always identical for information.)

  • Once added:
    screenshot
    β†ͺ️ Observe: original name kept, correct version of the moment. Tested and … additional rules are correctly present. βœ”οΈ

@contribucious
Copy link
Author

Context / Why it's desired initially

Read it …

Excluding all #$# rules (as currently in "trust level: low" filters, like Fanboy's Annoyances) IS problematic for "cookies" box.

For context, I needed to use the original version (in order not to have the limitation of trust level low) because many websites are broken by adding this (popular) filter in a built-in way (because of trust level low) due to this general action:
! [RULE-HERE] is excluded by "#$#" in exclusions-low.txt

Excluding #$# completely for security (and more) is understandable, but a lot of "annoyances" (the purpose of this filter in particular) of the "accept or not our cookies" type consist of the following:

  1. Hide the cookies box β€” using basic CSS rule(s);
  2. Play on the overflow CSS property of the body and/or html element (otherwise the site is very often not scrollable and therefore becomes "broken").

It's the second point that is problematic, because we have for example 316 matches (and many slight variants of style in fact) just for:
##body,html:style(height: auto !important; overflow: auto !important)
Like in:
! Rule "[DOMAIN-HERE]##body,html:style(height: auto !important; overflow: auto !important)" converted to: "[DOMAIN-HERE]#$#body,html { height: auto !important; overflow: auto !important }"

πŸ‘€ Converted but then … excluded:
Search:
is excluded by "#$#" in exclusions-low.txt
In:
https://raw.githubusercontent.com/AdguardTeam/FiltersRegistry/master/filters/ThirdParty/filter_122_FanboysAnnoyances/diff.txt

➑️ You'll see "More than 1000 matches"! Since a rule often has multiple domain names, that's a lot of potentially broken websites when using this popular filter in its AdGuard built-in version (I have seen this a few times myself). Instead of improving the user experience (thanks to the hard work of @ryanbr mainly regarding this list), it instead makes the latter worse on many websites (and this in a way unrelated to Ryan's work so). And currently, without signaling this to the user (for example via a ribbon like this or like that on the filter, but signaling "low" concerning its current trust level instead). Because otherwise, you might think that it's the filter itself (created by its author) that breaks a website, and possibly even report a detected issue on the author's GitHub repository for nothing (waste of time and energy), when it's actually … "the limited version of this filter in AdGuard" the problem here. Because of the trust level set on low for this one (which is not mentioned to the user when he add it at the very least, thinking to add "the one everyone uses", not a "partially broken" version of it πŸ˜‰).

Possible solutions.

  • Do not exclude rules β€” therefore strongly ensuring that they are OK on the security side β€” related to overflow CSS property for body and/or html elements (and some more complex ones for these elements unfortunately, search for example body,html:style) for the "low" level;
  • Add a new "low+" level for … "Annoyances" type filters that deal with cookie related dialogs;
  • Increase the trust level (trustLevel) of some built-in third-party (popular ones at least) "Annoyances" filters that deal with cookie related dialogs;
  • Inform, at least, the user via a ribbon "low" on the "trust level: low" filters (with possibly a tooltip regarding its potential effects);
  • Allow the user to set any built-in third-party filter as trusted by himself;
  • In any case, do not hijack β€” to a more limited version β€” the original URLs of the third-party filters if added manually via URL by the user when the latter has explicitly checked "trusted filter" at least. Or if hijacked, provide him with an AdGuard version of the same level of trust (i.e. converted/optimized but *not* more limited), although in this particular case hosting these risky versions of the filters can be problematic. Or, at the very least, let him know about this hijack and why that, in this case. Finally, it should be noted that AdGuard Browser extension is OK in this regard, as detailed in my issue.
  • … Yours? ☺️ I'm all ears!
    Β 
Sorry again for the length!

@Aydinv13
Copy link

@contribucious thank you for the clear design and very detailed description of your request, we considered it and decided to redo the principle of work in this place.

@contribucious
Copy link
Author

@Aydinv13 Great! Thank you! πŸ‘

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

No branches or pull requests

4 participants