Skip to content

Fix block list parser mis-parsing ABP cosmetic filters as domains - #2125

Open
SeanSith wants to merge 1 commit into
TechnitiumSoftware:masterfrom
SeanSith:fix-blocklist-cosmetic-filter-misparse
Open

Fix block list parser mis-parsing ABP cosmetic filters as domains#2125
SeanSith wants to merge 1 commit into
TechnitiumSoftware:masterfrom
SeanSith:fix-blocklist-cosmetic-filter-misparse

Conversation

@SeanSith

Copy link
Copy Markdown

Summary

BlockListZoneManager.ReadListFile recognizes #/! 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.IsDomainNameValid
before being enqueued, but the first word is never checked at all.

Adblock Plus cosmetic/procedural filters (##, #@#, #?#) attach a
domain list to a CSS/procedural selector that can contain natural
language, e.g. from easylist.to's fanboy-annoyance.txt:

tiktok.com##[aria-label="Scroll to the top"]
daily.dev#?#.bottom-0.fixed:has-text(This site uses cookies)

Neither line starts with # (the # is mid-line), ||, or @@||, so
they hit the hosts-format fallback. Splitting on whitespace picks out
the English word following the domain as the "second word" — to
and site respectively. A bare single-label name is syntactically
valid per IsDomainNameValid, so both get enqueued as blocked domains.
Once in the block zone, blocking is hierarchical, so this silently
blackholes the entire .to and .site TLDs — including, amusingly,
easylist.to itself.

I found this diagnosing a real installation where easylist.to (the
list's own host) and an unrelated .site-hosted domain were both
returning NXDOMAIN, traced via the block zone's EDNS debug extension
(source=block-list-zone; blockListUrl=...; domain=to) back to this
list.

Fix

Require the first word to already look like a domain name or IP
address before trusting the hosts-format split at all:

if (!DnsClient.IsDomainNameValid(firstWord) && !IPAddress.TryParse(firstWord, out _))
    continue;

This isn't specific to ##/#@#/#?# — it closes the same hole for
any current or future Adblock/uBlock/AdGuard syntax that isn't
explicitly handled (scriptlet/JS injection #%#, AdGuard's $$ HTML
filtering, etc.), since all of them put a character outside the
[a-zA-Z0-9-_/.] domain charset somewhere in that first token. No
marker 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 from
TechnitiumLibrary.Net, IPAddress.TryParse via socket.inet_pton)
and running it against 31 block lists from a real deployment's
configured set — before/after enqueued-domain counts:

List Before After Removed
easylist.to/easylist/fanboy-annoyance.txt 2499 2206 293 (incl. to, site)
filters.adtidy.org/.../15.txt 178145 178126 19
29 other configured lists (StevenBlack/hosts, AdAway, OISD, Hagezi, etc.) 0

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/site no longer appear in the parsed output
of fanboy-annoyance.txt after the change.

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.
@ShreyasZare

Copy link
Copy Markdown
Member

Thanks for the PR. Will check it soon.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants