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

Adding a particular filter list causes an indefinite crash #43170

Closed
2 of 6 tasks
igorskyflyer opened this issue Jan 8, 2025 · 3 comments
Closed
2 of 6 tasks

Adding a particular filter list causes an indefinite crash #43170

igorskyflyer opened this issue Jan 8, 2025 · 3 comments
Labels
OS/Android Fixes related to Android browser functionality

Comments

@igorskyflyer
Copy link

Description

Adding a particular ad-blocking filter list crashes the browser indefinitely.

Only tested on Android!

Steps to reproduce

  1. Go to Settings
  2. Go to Brave shields & privacy
  3. Go to Content filtering
  4. Click on Add custom filter list
  5. Add https://raw.githubusercontent.com/the-advoid/ad-void/main/AdVoid.Full.txt as the source

Actual result

The browser will crash indefinitely and cannot be used unless the data is cleared and the browser is reset to its initial state.

Expected result

The browser should not crash and be able to use the given filter list.

Reproduces how often

Easily reproduced

Brave version

Reproducible on:

  • stable (version 1.73.104)
  • beta (version 1.74.44)

Device

  • Brand/model: Samsung A55 5G
  • Android version: 14

Channel information

  • release (stable)
  • beta
  • nightly

Reproducibility

  • with Brave Shields disabled
  • with Brave Rewards disabled
  • in the latest version of Chrome

Miscellaneous information

No response

@igorskyflyer igorskyflyer added the OS/Android Fixes related to Android browser functionality label Jan 8, 2025
@TEMP-ad
Copy link

TEMP-ad commented Jan 8, 2025

The reason for the list to crash, is the generic procedural cosmetic :has-text() in
https://github.com/the-advoid/ad-void/blob/22603e91b887918c290d0dcc578b41cc2d318943/AdVoid.Full.txt#L4893
https://github.com/the-advoid/ad-void/blob/22603e91b887918c290d0dcc578b41cc2d318943/rules/core/cosmetic-global.txt#L567
https://github.com/the-advoid/ad-void/blob/22603e91b887918c290d0dcc578b41cc2d318943/AdVoid.Core.txt#L4627

The problem is you are making it generic, and that's something not even uBlock allows, you can read it here in the first point: https://github.com/gorhill/ublock/wiki/Procedural-cosmetic-filters#important

It doesn't crash in uBlock, and doesn't give an error, but if you test a p with Advertisement, it will not get hidden by uBlock, it will just get ignored.

So make it specific or remove it.


@antonok-edm in Brave the crash happens in Android and Desktop, because it is trying to parse it, and don't just ignore it as any invalid rule.

The crash happens if you add it even as a custom rule, so no need for a list.
In Brave Desktop (windows) it will crash and don't save the list/custom rule so it will not keep crashing the browser which is not good but the browser will work after it gets restarted so maybe the user will not try to add the same list/rule after that; but in Android, the changes stay and it will just crash the whole thing until you clear the data, so this problem is worst in Android for sure.

Of course nobody should make generic procedural cosmetic rules, but of course if they do, it shouldn't cause a crash and just get discarded.

@TEMP-ad
Copy link

TEMP-ad commented Jan 9, 2025

Well, this is confusing, report an issue, I did the 'job' of reporting the exact issue, which means regardless of Brave's bug and crashing for trying to parse the rule, it can and should be easily fixed by removing a useless rule that does nothing anyway. But then OP doesn't even say a word, doesn't fix the list, but I guess it is more important to comment how Edge Canary can load uBlock on reddit.

Awkward.

I hope not many Brave users intend to use the-advoid or didn't use it in the past, but then I guess I care too much about some random people who might install a list that will crash their browser until their reset the app's data.

@igorskyflyer
Copy link
Author

@TEMP-ad, as mentioned in the other issue, thank you for heads up and for figuring out what caused this issue in the first place.

I do have a life, it's Christmas (Orthodox) time in my place, and I am not 24/7 by my computer since AdVoid filters need to be compiled, it's not just a simple text edit. The given rule was a part of the experimental filters but was promoted to the Core list by mistake.

What also needs to be addressed is that Brave's adblocker as all other adblockers should ignore non-valid rules and not crash immediately (and indefinitely) when it stumbles upon one.

And what's wrong with finding a browser trick and sharing it with the world?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OS/Android Fixes related to Android browser functionality
Projects
None yet
Development

No branches or pull requests

2 participants