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

Issue with $redirect when the script is upgraded to HTTPS. #26415

Closed
ghost opened this issue Nov 1, 2022 · 7 comments · Fixed by brave/brave-core#15754
Closed

Issue with $redirect when the script is upgraded to HTTPS. #26415

ghost opened this issue Nov 1, 2022 · 7 comments · Fixed by brave/brave-core#15754

Comments

@ghost
Copy link

ghost commented Nov 1, 2022

Hi @pes10k
There is a big, not so big, kind of big issue with $redirect or $redirect-rule filters when the Network element is being upgraded to https. I feel I found it, even if it was in plain sight all this time.

Basically, the problem is that the script upgraded to https is not even getting blocked, it is allowed completely when a $redirect rule is being applied to it.
It acts like if it was whitelisted which is not good, especially if we talk about google scripts being the most common items that get redirected.

Easiest way to see:

  1. Add this as a custom filter:
    ||google-analytics.com/analytics.js$script,redirect=google-analytics.com/analytics.js This has to be done because of the problem with Brave not doing the $redirect as it should, so it won't work by default for you as it does for me since I fixed/found workarounds for all those issues on my side.

  2. go to http://markkystreams.com/ (easiest website I found to do this test this)

  3. Open shields and it will say google-analytics.com/analytics.js was upgraded and blocked.

  4. Open DevTools and you will see analytics is in the source list, so Shields information is not accurate since the script wasn't blocked, because if it was, it wouldn't be in the list and it wasn't redirected since the script is the same analytics script used by the page, not the uBlock one.

  5. Turn Upgrade connections to HTTPS Off, and then you will see the script properly redirected with the uBlock google-analytics resource.

I had to solved this for now (hopefully) by turning https upgrade completely off in Preferences file and now Brave adblocker works beautifully, since no scripts are being allowed like that anymore, and I don't have to see uBlock telling me about it and doing the job.

Since I fixed the issues with $redirect and my Brave adblocker works "perfect" with the redirect feature, I see this problem a lot, of course, normal Brave users will not have this problem since the script will just get blocked by Brave today (unless they do the same I did). But whenever Brave supports the priority integer for the redirect directives and uBlock lists are more compatible with Brave, then normal users will get this problem and many http scripts being upgraded will just be allowed to connect even if they should not do that at all.

I mean, it doesn't affect me now I did the fixes on my side but still reporting what I think is a not good but not the most terrible bug.

Thank you and have a good day!

@ghost ghost added OS/Android Fixes related to Android browser functionality OS/Desktop labels Nov 1, 2022
@ghost
Copy link
Author

ghost commented Nov 1, 2022

This is a screenshot of the NextDNS with the website I gave for the test, just to show you how the script/connection is being allowed, so it is not a visual bug, my imagination or I want to bother you and make you waste your time with this.

I mean, I just had to turn off the https upgrade feature globally so this doesn't happen anymore, but hope it can get fixed eventually, since it is clearly a bug or miss-behavior by the adblocker handling redirection with a http script upgraded to https.

image

@antonok-edm
Copy link
Collaborator

@TheVampireInLoveWithTheCorpsesBlood Thanks for another helpful report; the fix should be in brave/brave-core#15754. Great to have your eyes on these kind of things 😄

@ghost
Copy link
Author

ghost commented Nov 2, 2022

@antonok-edm Oh nice! Thanks for the quick fix and the amazing work!

@kjozwiak
Copy link
Member

kjozwiak commented Nov 3, 2022

The above requires 1.46.91 or higher for 1.46.x verification 👍

@stephendonner
Copy link

stephendonner commented Nov 4, 2022

Verified PASSED using

Brave 1.46.92 Chromium: 107.0.5304.91 (Official Build) beta (64-bit)
Revision 3d5948960d62418160796d5831a4d2d7d6c90fa8-refs/branch-heads/5304@{#1097}
OS Windows 10 Version 22H2 (Build 19045.2130)

Steps:

  1. installed 1.46.92
  2. launched Brave
  3. opened brave://settings/shields/filters
  4. added ||google-analytics.com/analytics.js$script,redirect=google-analytics.com/analytics.js to the Create custom filters textfield
  5. clicked Save changes
  6. loaded http://markkystreams.com/
  7. clicked on the Shields icon in the URL bar
  8. confirmed Google Analytics shows up in the 11 Trackers & ads blocked section
  9. toggled Upgrade connections to HTTPS to Off
  10. reloaded the page
  11. opened Developer Tools
  12. chose the Network tab
  13. selected analytics.js and looked at the Preview pane

Confirmed analytics.js was the uBlock Origin-based one

example example example example
image image image image

NOTE: I attempted verification on macOS, but the site itself doesn't load, even using vanilla Chrome builds, on macOS; see this Slack thread for more info 👍

/cc @LaurenWags for the above macOS note

@Uni-verse
Copy link
Contributor

Uni-verse commented Nov 22, 2022

Verified on Samsung Galaxy S21 & Samsung Galaxy Tab S7 using the following build(s):

Brave	1.46.110 Chromium: 107.0.5304.110 (Official Build) beta (64-bit) 
Revision	2a558545ab7e6fb8177002bf44d4fc1717cb2998-refs/branch-heads/5304@{#1202}
OS	Android 12; Build/SP1A.210812.016

Using the STR in the test plan in brave/brave-core#15754 (comment)

Added custom adblock filter ||google-analytics.com/analytics.js$script,redirect=google-analytics.com/analytics.js

screenshot-1669158098427

  • Verified that http://www.google-analytics.com appears in trackers and ads blocked setting rather than the Upgrade connections to HTTPS setting.
Example Example
screenshot-1669157621199 screenshot-1669157636539
  • Verified that the analytics.js entry in the network panel has contents (Response tab) starting with the uBlock Origin copyright header.

Screen Shot 2022-11-22 at 5 56 42 PM

@kjozwiak
Copy link
Member

@Uni-verse looks like the above was verified on Android as per #26415 (comment)? Adding the QA/Pass labels, as the above looks completed. Please correct me if I'm wrong.

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

Successfully merging a pull request may close this issue.

5 participants