brave/adblock-rust

$removeparam can crash Brave easily

Closed this issue ยท 2 comments

Hi,

Crashes are easy to reproduce, they all happen 100% of the time. I don't know why, if it's meant to crash because of the way removeparam works or it can be fixed, but I hope my report can help to improve it.

I tested in my normal installation and a new user-data with no extensions or anything, everything default (only adblock in aggressive mode) in latest Nightly as today: 1.49.15

Example 1:
||aliexpress.com^$removeparam=gatewayAdapt

Now go to aliexpress.com by address bar or search and it will insta-crash, and if you add the rule after going to aliexpress, pretty much Brave crashes on any click you do in Aliexpress.

Example 2:
add any of these rules ||startpage.com^$removeparam=sc

||startpage.com^$removeparam=t
||startpage.com^$removeparam=cat
||startpage.com^$removeparam=abp
||startpage.com^$removeparam=lui
||startpage.com^$removeparam=qloc

Startpage by default uses Post instead of Get to do the searches, if you use :sp test you will notice the removeparam works great, but now, go to settings and change the HTTP request method from Post to Get and save the settings, Brave should crash immediately.

Example 3:
||youtube.com^$removeparam=feature crashes Brave, feature parameter is added when you right click and copy video URL.

Example 4:
$document,removeparam=utm_campaign if you go to AMD website from a campaign email or something like that like this one , and it is not excluded by using domain=~amd.com it crashes Brave

Based on my comments on my filter list, it also happens with $document,removeparam=utm_source and $document,removeparam=utm_term where I excluded AMD domain.

Example 5:
Crashes or Conflict with Query Filter, if you go to https://fmarier.github.io/brave-testing/query-filter.html
and you have in your list or custom filters $document,removeparam=fbclid and you click in pretty much everything everything but the first link Cross-origin navigation, then Brave will crash.

After this, I excluded all rules that are already hardcoded in Query Filter so I don't know if any other will cause crash, also, it seems it is when it 'redirects' since it doesn't crash in the first one which is a normal page with https://fmarier.org/query-filter/?fbclid=1234 on it.

These are the examples of crashes I have noticed while using $removeparam I have also encountered other crashes, like once I had to login to a Yahoo account and when I did the whole 2FA and logged in, it crashed, but I don't know which rule(s) caused it, so once I turned off my $removeparam list it worked. But it had to be one generic rule since Adguard only has rules for yahoo.co.jp and not for .com

Today I also encountered a crash when I was trying to find some JS guide, don't know which website was it or what exactly was the searching query to go and check every result and see which one crashes but I am sure it was the $removeparam, so it is impossible to find it but

But I also found an Ad address like this from Startpage can crash Brave, so maybe I accidentally clicked an Ad or something: https://www.google.com/aclk?sa=l&ai=CMCkQWtm_Y7j_ENaPtOUP3p2r0AjYyq6SbtGSmebIC46ev9wHEAEgv_7VF2DJBpABBaAB4qrr3APIAQHIAxuqBF5P0F95K6Lgegs0kD29u1vWHhL4Z_Y3VdJztx1vI95Fi2yJ3FjpP51AM-srbwELwUBm9i5wGBmIz33Wz94z-54bhAcsqi-2QEajroKtBu8sLMYNmPlGzq54JCX91xu5wASB-LmG8QKIBe6XvqAkoAZRgAeG1ZQjkAcBqAfU0huoB6a-G6gHuZqxAqgH89EbqAfu0huoB_-csQKoB8rcG6gHj6mxAqgHu6SxAqgH2KaxAqgHkaqxAqgH26qxAqgH0KqxAqAIk8M-sAgF0ggSEAEgpIECMgOHwAI6AgACQgECmgkhaHR0cHM6Ly9zZW50cnkuaW8vZm9yL2phdmFzY3JpcHQvsQk25dzhjYm1F7kJr4KT-2hHVjj4CQHgCwG4DAHoDAOqDQJVU4IUHAgBEhhkZXRlY3QgamF2YXNjcmlwdCBlcnJvcnOIFAHQFQGYFgH4FgGAFwGSFwgSBggBEAMYNg&num=1&sig=AOD64_0P_kvSMKlriPbpe_CR8pSB5zl7UQ&adurl=https://monitor.ppcprotect.com/v1.0/template%3Faccid%3D11434%26kw%3Ddetect%2520javascript%2520errors%26mt%3Db%26nw%3Ds%26cpn%3D9731804142%26devi%3Dc%26devm%3D%26locp%3D1024403%26loci%3D%26pl%3D%26cr%3D428112714701%26adp%3D%26ruid%3D15048584936101084035%26sadt%3D%26url%3Dhttps://sentry.io/for/javascript/%253Futm_source%253Dgoogle%2526utm_medium%253Dcpc%2526utm_campaign%253D9731804142%2526utm_content%253Ds%2526utm_term%253Ddetect%252520javascript%252520errors%2526device%253Dc%2526gclid%253D%257Bgclid%257D&q=&rurl=https%3A%2F%2Fwww.startpage.com%2F&nm=6&nx=150&ny=24&is=632x129

which is the same exact case as AMD page, if $document,removeparam=utm_campaign is in the list.

Hope this can help to make $removeparam more stable!
Thanks for your work.

Thanks for the detailed report! We hadn't seen this happening before.

I'll close this issue and use brave/brave-browser#27791 to track it instead, since it's actually an issue in brave-core rather than adblock-rust.

I was able to make a PR that should fix it: brave/brave-core#16654

Thanks! I didn't know where to report it so I did it in the same place with my other issue.
Since removeparam was pushed to Stable it's what worries me, especially when/if people will start reporting Brave randomly crashing. I should have reported it sooner! since I got the crashes since the first Nightly when it was released.

The 'good' (not really) thing is since most people will just add the Adguard list and most rules will not work as I explained in my other issue, then that will avoid most crashes, but yeah, it's good to know it was an 'easy'/'quick' fix to make it stable!

Thanks for your work and the quick fix as usual! ๐Ÿ‘Œ
I always seem to find the issues, like with websockets or the redirect issue with http connections! ๐Ÿ˜