From 98fbeb6791921f14868e933fa997bf130303e9f2 Mon Sep 17 00:00:00 2001 From: Jakub Zych Date: Tue, 27 Oct 2026 15:31:56 +0100 Subject: [PATCH] fix: resolve and the 404 report refuse NUL and invalid UTF-8 addresses (WR-12) A decoded path such as /%00 or /%FF reached Postgres as text and answered 500 with an error log line per request. parseTarget refuses a path or query that is not valid UTF-8 or holds NUL (422), NotFoundPath does the same for the logged address, and a referer is cleaned of both. --- classes/notfound.go | 8 ++++++-- classes/resolver.go | 24 +++++++++++++++++++++++- redirects_test.go | 19 +++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/classes/notfound.go b/classes/notfound.go index e2ed304..8f1d92e 100644 --- a/classes/notfound.go +++ b/classes/notfound.go @@ -76,7 +76,11 @@ func NotFoundPath(raw string) (string, error) { if p == "" { p = "/" } - return joinQuery(p, u.RawQuery), nil + out := joinQuery(p, u.RawQuery) + if !safeText(out) { + return "", ErrNotFoundURL + } + return out, nil } // capWarned keeps when the row-cap warning was last logged. @@ -94,7 +98,7 @@ func (s *Service) ReportNotFound(ctx context.Context, rawURL, referer string) er if err != nil { return err } - referer = strings.TrimSpace(referer) + referer = strings.TrimSpace(strings.ToValidUTF8(strings.ReplaceAll(referer, "\x00", ""), "")) if len(referer) > MaxRefererBytes { referer = referer[:MaxRefererBytes] for len(referer) > 0 && !utf8ValidTail(referer) { diff --git a/classes/resolver.go b/classes/resolver.go index 8f1dc7f..7e867f8 100644 --- a/classes/resolver.go +++ b/classes/resolver.go @@ -6,6 +6,7 @@ import ( "net/url" "sort" "strings" + "unicode/utf8" "git.golem15.com/golem15/sm-seo-plugin/models" "git.golem15.com/golem15/summercms/modules/cabana" @@ -215,7 +216,28 @@ func parseTarget(raw string) (string, url.Values, error) { if path == "" { path = "/" } - return path, u.Query(), nil + query := u.Query() + if !safeText(path) || !safeText(u.RawQuery) { + return "", nil, ErrBadURL + } + for k, vs := range query { + if !safeText(k) { + return "", nil, ErrBadURL + } + for _, v := range vs { + if !safeText(v) { + return "", nil, ErrBadURL + } + } + } + return path, query, nil +} + +// safeText reports whether s can be stored and compared as Postgres text: +// valid UTF-8 without NUL bytes. A decoded path such as /%00 or /%FF is +// not (WR-12). +func safeText(s string) bool { + return utf8.ValidString(s) && !strings.ContainsRune(s, 0) } // contentLocale returns the record's description in locale, else its first diff --git a/redirects_test.go b/redirects_test.go index a0e138d..8b94fb6 100644 --- a/redirects_test.go +++ b/redirects_test.go @@ -656,6 +656,25 @@ func TestNotFoundLog(t *testing.T) { if rec := env.report(t, secret, map[string]string{"url": "/" + strings.Repeat("a", 2100)}); rec.Code != http.StatusUnprocessableEntity { t.Fatalf("report of a long URL = %d, want 422", rec.Code) } + // A decoded NUL or invalid UTF-8 byte is a 422, never a database error + // (WR-12); so is the public resolve answer. + // The 404 log keeps the query escaped, so only the path is refused. + for _, bad := range []string{"/%00", "/%FF", "/a?x=%00", "/a?%FF=1"} { + if rec := env.report(t, secret, map[string]string{"url": bad}); strings.HasPrefix(bad, "/%") && rec.Code != http.StatusUnprocessableEntity { + t.Fatalf("report of %s = %d %s, want 422", bad, rec.Code, rec.Body.String()) + } else if rec.Code >= 500 { + t.Fatalf("report of %s = %d %s", bad, rec.Code, rec.Body.String()) + } + if rec := env.call(t, http.MethodGet, "/_seo/api/v1/resolve?url="+urlQueryEscape(bad), "", nil); rec.Code != http.StatusUnprocessableEntity { + t.Fatalf("resolve of %s = %d %s, want 422", bad, rec.Code, rec.Body.String()) + } + } + if rec := env.report(t, secret, map[string]string{"url": "/bad-referer", "referer": "https://ref.example/\x00\xff"}); rec.Code != http.StatusNoContent { + t.Fatalf("report with a NUL in the referer = %d %s, want 204", rec.Code, rec.Body.String()) + } + if err := env.db.Exec(`DELETE FROM golem15_seo_notfound`).Error; err != nil { + t.Fatal(err) + } if rec := env.report(t, secret, map[string]string{"url": "/missing", "referer": "https://ref.example/a"}); rec.Code != http.StatusNoContent || rec.Header().Get("X-Robots-Tag") != "noindex" { t.Fatalf("report = %d %v, want 204 with X-Robots-Tag", rec.Code, rec.Header()) }