Fix block list parser mis-parsing ABP cosmetic filters as domains - #2125
Open
SeanSith wants to merge 1 commit into
Open
Fix block list parser mis-parsing ABP cosmetic filters as domains#2125SeanSith wants to merge 1 commit into
SeanSith wants to merge 1 commit into
Conversation
The block-list-zone parser only recognizes #/! comments and ||domain^ /
@@||domain^ Adblock network-filter syntax; anything else falls through
to a hosts-format guess that splits the line on whitespace and treats
the second word as the domain, without validating the first word at
all.
Adblock Plus cosmetic/procedural filters (##, #@#, #?#) embed natural
language selector text alongside the domain list, e.g.:
tiktok.com##[aria-label="Scroll to the top"]
daily.dev#?#.bottom-0.fixed:has-text(This site uses cookies)
These don't start with #, so they hit the hosts-format fallback, which
splits on whitespace and picks out plain English words ("to", "site")
as the "second word" domain. Since a bare single-label name is
syntactically valid, it gets enqueued and blocks the entire .to / .site
TLD once inserted into the block zone.
Requiring the first word to itself be a valid domain name or IP address
before trusting the split closes this for the whole family of ABP/uBO/
AdGuard syntax (cosmetic filters, scriptlet/JS injection, $$ HTML
filtering), since all of them introduce characters outside the
[a-zA-Z0-9-_/.] domain charset into that first token - no need to
special-case each marker.
Verified by replicating this parsing logic against 31 block lists from
a real deployment's configured set, including two Adblock-format lists
(easylist.to's fanboy-annoyance.txt and filters.adtidy.org's Chromium
filter list) and several genuine hosts-format lists (StevenBlack,
AdAway, etc.). Zero regressions on the hosts-format lists; 293 garbage
entries removed from fanboy-annoyance.txt (including the exact "to" and
"site" entries that were blackholing those TLDs), and 19 similar
garbage entries removed from the still-in-use adtidy list.
Member
|
Thanks for the PR. Will check it soon. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
BlockListZoneManager.ReadListFilerecognizes#/!comments and||domain^/@@||domain^Adblock network-filter syntax explicitly.Any other line falls through to a "hosts file format" guess: split the
line on whitespace, and if there's a second word, treat that as the
domain. The second word is validated with
DnsClient.IsDomainNameValidbefore being enqueued, but the first word is never checked at all.
Adblock Plus cosmetic/procedural filters (
##,#@#,#?#) attach adomain list to a CSS/procedural selector that can contain natural
language, e.g. from easylist.to's
fanboy-annoyance.txt:Neither line starts with
#(the#is mid-line),||, or@@||, sothey hit the hosts-format fallback. Splitting on whitespace picks out
the English word following the domain as the "second word" —
toand
siterespectively. A bare single-label name is syntacticallyvalid per
IsDomainNameValid, so both get enqueued as blocked domains.Once in the block zone, blocking is hierarchical, so this silently
blackholes the entire
.toand.siteTLDs — including, amusingly,easylist.toitself.I found this diagnosing a real installation where
easylist.to(thelist's own host) and an unrelated
.site-hosted domain were bothreturning NXDOMAIN, traced via the block zone's EDNS debug extension
(
source=block-list-zone; blockListUrl=...; domain=to) back to thislist.
Fix
Require the first word to already look like a domain name or IP
address before trusting the hosts-format split at all:
This isn't specific to
##/#@#/#?#— it closes the same hole forany current or future Adblock/uBlock/AdGuard syntax that isn't
explicitly handled (scriptlet/JS injection
#%#, AdGuard's$$HTMLfiltering, etc.), since all of them put a character outside the
[a-zA-Z0-9-_/.]domain charset somewhere in that first token. Nomarker allowlist to maintain.
Testing
There's no test project in this repo, so I verified by re-implementing
the exact parsing logic (comment/
||/@@||/hosts-format branches,IsDomainNameValid's character rules pulled fromTechnitiumLibrary.Net,IPAddress.TryParseviasocket.inet_pton)and running it against 31 block lists from a real deployment's
configured set — before/after enqueued-domain counts:
to,site)Zero regressions on every genuine hosts-format list; the fix only
strips entries from the two lists that use Adblock cosmetic-filter
syntax, and confirmed
to/siteno longer appear in the parsed outputof
fanboy-annoyance.txtafter the change.