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.
This commit is contained in:
@@ -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) {
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user