diff --git a/CHANGELOG.md b/CHANGELOG.md index 23e85e462..531b9c674 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -68,6 +68,8 @@ See also the [v0.107.78 GitHub milestone][ms-v0.107.78]. - The parsing of the `ech` parameter in DNS rewrite rules for the HTTPS record type ([#8276]). +- Validation of the `answer` field in DNS rewrite rules in case it is represented as CNAME. + - Blocked services check on the Custom filtering rules page does not work properly without specifying of a client. [#7955]: https://github.com/AdguardTeam/AdGuardHome/issues/7955 diff --git a/internal/filtering/rewritehttp.go b/internal/filtering/rewritehttp.go index b619ca120..cbac4ab48 100644 --- a/internal/filtering/rewritehttp.go +++ b/internal/filtering/rewritehttp.go @@ -74,8 +74,6 @@ func (d *DNSFilter) handleRewriteAdd(w http.ResponseWriter, r *http.Request) { err = rw.normalize(ctx, l) if err != nil { - // Shouldn't happen currently, since normalize only returns a non-nil - // error when a rewrite is nil, but be change-proof. aghhttp.ErrorAndLog(ctx, l, r, w, http.StatusBadRequest, "normalizing: %s", err) return @@ -173,8 +171,6 @@ func (d *DNSFilter) handleRewriteUpdate(w http.ResponseWriter, r *http.Request) err = rwAdd.normalize(ctx, l) if err != nil { - // Shouldn't happen currently, since normalize only returns a non-nil - // error when a rewrite is nil, but be change-proof. aghhttp.ErrorAndLog(ctx, l, r, w, http.StatusBadRequest, "normalizing: %s", err) return diff --git a/internal/filtering/rewritehttp_test.go b/internal/filtering/rewritehttp_test.go index f3734fd7a..fd25404ea 100644 --- a/internal/filtering/rewritehttp_test.go +++ b/internal/filtering/rewritehttp_test.go @@ -50,6 +50,7 @@ const ( decodeUpdateErrorMsg = decodeMsg + " filtering.rewriteUpdateJSON\n" ) +// TODO(m.kazantsev): Improve maintainability. func TestDNSFilter_HandleRewriteHTTP(t *testing.T) { t.Parallel() @@ -66,6 +67,7 @@ func TestDNSFilter_HandleRewriteHTTP(t *testing.T) { updAnswer = "upd.rewrite" invDomain = "inv.local" invAnswer = "inv.rewrite" + invalidDomain = "invalid_domain" ) testRewrites := []*rewriteJSON{ @@ -148,6 +150,21 @@ func TestDNSFilter_HandleRewriteHTTP(t *testing.T) { wantStatus: http.StatusBadRequest, wantBody: decodeErrorMsg, wantList: testRewrites, + }, { + name: "add_error_invalid_cname", + url: addURL, + method: http.MethodPost, + reqData: rewriteJSON{ + Domain: addDomain, + Answer: "invalid_domain", + Enabled: aghalg.NBTrue, + }, + wantConfMod: false, + wantStatus: http.StatusBadRequest, + wantBody: `normalizing: invalid CNAME target "invalid_domain": bad domain name ` + + `"invalid_domain": bad top-level domain name label "invalid_domain": bad top-level ` + + `domain name label rune '_'` + "\n", + wantList: testRewrites, }, { name: "delete", url: deleteURL, diff --git a/internal/filtering/rewrites.go b/internal/filtering/rewrites.go index 6e9b247e0..71a2a5494 100644 --- a/internal/filtering/rewrites.go +++ b/internal/filtering/rewrites.go @@ -10,6 +10,7 @@ import ( "github.com/AdguardTeam/golibs/errors" "github.com/AdguardTeam/golibs/logutil/slogutil" + "github.com/AdguardTeam/golibs/netutil" "github.com/miekg/dns" ) @@ -93,6 +94,15 @@ func (rw *LegacyRewrite) normalize(ctx context.Context, l *slog.Logger) (err err ip, err := netip.ParseAddr(rw.Answer) if err != nil { l.DebugContext(ctx, "normalizing legacy rewrite", slogutil.KeyError, err) + + // Not an IP address, treat as CNAME target, but validate as a domain + // name first. + err = netutil.ValidateDomainName(rw.Answer) + if err != nil { + // Use capital letters, as the error message is shown to the user. + return fmt.Errorf("invalid CNAME target %q: %w", rw.Answer, err) + } + rw.Type = dns.TypeCNAME return nil