AdguardTeam / AdguardTeam/HostlistCompiler

Validator bypasses hostname checks for ||domain-style rules missing a ^ terminator

未关闭
#129 0 条评论 0 个 reaction 已指派 1 人 已被 @maximtop 认领 在 GitHub 查看
Priority: P4
主要语言
JavaScript
星标
232
派生
30
PR 合并指标
30 天内没有已合并 PR

描述

The "uBlock₀ filters – Badware risks" list (uAssets [`badware.txt`](https://github.com/uBlockOrigin/uAssets/blob/master/filters/badware.txt), sourced into AdGuard DNS via [`HostlistsRegistry` filter #50](https://github.com/AdguardTeam/HostlistsRegistry/blob/main/assets/filter_50.txt)) contains this rule, targeting phishing lookalikes of `roblox.com` such as `roblox.com.gr`:

```
||roblox.com.$all
```

The trailing dot with no `^` terminator is intentional, valid uBO/Adblock syntax: it requires something to follow `roblox.com.`, which real Roblox subdomains never have (they precede `roblox.com`, not follow it) but arbitrary lookalike TLDs do (`roblox.com.gr`, `roblox.com.br`, etc.). This lets one rule catch the whole class of `roblox.com.*` typosquats without matching the real domain or its real subdomains — using `^` instead would only match `roblox.com` exactly, which is the opposite of what's needed here.

`HostlistCompiler` correctly strips the unsupported `$all` modifier for DNS use, producing:

```
||roblox.com.
```

but this compiled rule ends up blocking `clientsettingscdn.roblox.com` (legitimate Roblox domain) on AdGuard DNS (confirmed via the "Check the domain" tool) while leaving `roblox.com` and other subdomains unaffected — behavior the source uBO syntax was never meant to produce.

**Root cause**

In `src/transformations/validate.js`, `validAdblockRule` has this check:

```js
// Check if the pattern does not start with the domain prefix and does not contain a domain separator
if (!_.startsWith(props.pattern, DOMAIN_PREFIX) || sepIdx === -1) {
return true;
}
```

where `DOMAIN_PREFIX = '||'` and `DOMAIN_SEPARATOR = '^'`. Since `||roblox.com.` starts with `||` but contains no `^` anywhere, `sepIdx === -1`, and the function returns `true` (valid) immediately — bypassing every hostname-level check that follows.

More importantly, this is a case where the validator's own model doesn't match the DNS matching engine's behavior. The compiler's `EXACT_DOMAIN_PATTERN` regex and its comments show awareness of trailing-dot FQDN notation, but only for `^`-terminated rules (e.g. `||example.org.^`), where `extractDomainPattern` explicitly normalizes the trailing dot away, treating it as equivalent to no dot at all (line 228-229: *"Allow trailing dot as absolute FQDN marker by normalizing it away for validation"*). But per my testing of both `urlfilter` and `dnsfilter`, a trailing dot *without* `^` is not equivalent to no dot — it changes the rule's matching scope entirely (requiring something to follow, rather than matching the bare domain). A rule in this form isn't a broken domain rule that happens to skip validation; it's a different, valid rule type — a suffix/TLD-variant matcher — that the validator has no model for at all, so it can't verify whether the DNS-compiled output does what the original rule intended.

**Suggested fix**

Rules starting with `DOMAIN_PREFIX` but lacking `DOMAIN_SEPARATOR` shouldn't get an automatic pass. At minimum, apply the same `checkChars`/hostname sanity checks used for anchored rules; ideally, recognize trailing-dot-no-caret patterns as their own category and verify their DNS-compiled behavior actually matches the semantics they have in uBO (only matching hostnames with a suffix beyond the given domain), rather than silently passing them through as if they were ordinary domain-anchored rules.

If this is a known/expected side effect of how non-DNS-oriented filter rules get converted for DNS matching, consider adding a warning in the AdGuard DNS UI when a list contains rules that upstream maintainers have explicitly stated aren't meant for DNS use — uAssets maintainers told me directly in https://github.com/uBlockOrigin/uAssets/issues/33636: *"Filters in uAssets are not meant to be used in DNS. Remove that list in your DNS settings."* Without a warning, users keep filing reports against uAssets for behavior that's actually specific to the DNS conversion, creating avoidable maintenance burden for filter list maintainers who have no way to fix it on their end without weakening what the rule is designed to catch.

Image

贡献指南

这个仓库没有索引到贡献指南

评估

这个 Issue 还没有评估数据。

把新 issue 发到你的邮箱

精选适合新手参与的 GitHub issue 摘要。