Skip to content
This repository has been archived by the owner on Sep 19, 2020. It is now read-only.

Subdomains of blocklisted domains are incorrectly displayed as explicitly blocked #284

Open
9 of 10 tasks
xofe opened this issue Aug 11, 2020 · 5 comments
Open
9 of 10 tasks

Comments

@xofe
Copy link

xofe commented Aug 11, 2020

Prerequisites

  • I performed a cursory search of the issue tracker to avoid opening a duplicate issue
  • This is not a support issue or a question
    • Support issues and questions are handled at /r/uMatrix
  • I tried to reproduce the issue when...
    • uMatrix extension is wholly disabled or not installed
    • uMatrix is the only extension
    • uMatrix with default lists/settings
    • using a new, unmodified browser profile
  • I am running the latest version of uMatrix
  • I checked the documentation to understand that the issue I report is not a normal behavior
  • I used the logger to rule out that the issue is caused by my ruleset

Description

Subdomains of blocklisted domains are incorrectly displayed as explicitly blocked, rather than inherited blocked.
This does not occur in v1.4.0. git bisect blames gorhill/uMatrix@9b29230.

A specific URL where the issue occurs

http://xofe.mm.st/umatrix-subdomain-test/

Site source:

<img src="//test-domain.invalid/">
<img src="//domain-not-in-list.test-domain.invalid/">
<img src="//domain-in-list.test-domain.invalid/">

Using hosts list: http://xofe.mm.st/umatrix-subdomain-test/blocklist.txt

List source:

0.0.0.0 test-domain.invalid
0.0.0.0 domain-in-list.test-domain.invalid

Steps to Reproduce

  1. Open clean browser profile and install uMatrix latest.
  2. Add hosts list http://xofe.mm.st/umatrix-subdomain-test/blocklist.txt.
  3. Navigate to http://xofe.mm.st/umatrix-subdomain-test/.
  4. Open uMatrix popup and notice domain-not-in-list.test-domain.invalid is dark red (explicit block) rather than pale red (inherited block)

Ruleset

Default ruleset

Supporting evidence

Version 1.4.0:
1 4 0

Version 1.4.1b6:
1 4 1

Your environment

  • uMatrix version: 1.4.1b6 (b26b3bb)
  • Browser Name and version: Firefox 80
@gorhill
Copy link
Member

gorhill commented Aug 11, 2020

Pretty sure it's a side effect of now using HNTrie. Off the top of my head, this should be fixable by ensuring the hostname wholly matches here, i.e. if ubiquitousBlacklistRef.matches() returns > 0, it's an inherited block.

@gorhill
Copy link
Member

gorhill commented Aug 13, 2020

@xofe You have contributed pull requests in the past, would you be willing to fix the issue here and submit a pull request?

@xofe
Copy link
Author

xofe commented Aug 13, 2020

Sure, I can do that.

Checking ubiquitousBlacklistRef.matches() is 0 fixes the specfic case I wrote above, but would mean domain-in-list.test-domain.invalid is not explicitly blocked after the change (despite appearing in blocklist), as HNTrie currently includes only test-domain.invalid, not any subdomains.

To fix this completely, I think it would require changes in HNTrie too. Namely, reverting part of gorhill/uBlock@adabb56d for uMatrix so that blocklisted subdomains of already blocked domains are still added to the trie, and removing the early exit when a domain match is found here: https://github.com/gorhill/uMatrix/blob/0bcb766/src/js/hntrie.js#L222-L224

Does this sound right to you?

@gorhill
Copy link
Member

gorhill commented Aug 13, 2020

blocklisted subdomains of already blocked domains are still added to the trie

I think this should be part of another issue. The fix to this is also not so trivial because this would require to also revisit the WASM code.

Also, I am also not sure this needs fixing, this can be argued that there is little to be gained to include all subdomains given that uMatrix is naturally meant to be used in a block-all/allow-exceptionally manner.

@xofe
Copy link
Author

xofe commented Aug 14, 2020

Fair points, I'll leave that part alone then.

I've made a pull request for this (and a few other minor fixes) at gorhill/uMatrix#1015, here's the popup after the change:

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

No branches or pull requests

2 participants