fix(17): T-17-15 apply the frontend_url rewrite only for a safe http(s) URL

- models.AllowedFrontendURL: empty, or an absolute http(s) URL with a host and without whitespace, control characters, a double quote, <, > or & - classes.Rewrite leaves the document unchanged for a refused value, inserts through EscapeText/EscapeAttr and keeps a loc or href that would not stay an http(s) URL - TestRewriteUnsafeFrontendURL, TestAllowedFrontendURL and a stored-row check in TestSitemapImportSmoke
2026-10-13 00:57:02 +02:00
parent cf5c40e442
commit b9804ddf17
6 changed files with 241 additions and 20 deletions
--- a/README.md
+++ b/README.md
@@ -68,7 +68,7 @@ Each URL is written in the PHP order (`classes.Generator`):

 A registry item is resolved with its URL pattern, its nesting flag and the settings' frontend default locale as the unprefixed locale. A resolved item with a URL is written, then its sub-items once each with their own URL. A list type (a resolved item without a URL, such as all posts) writes each sub-item once per alternate, with that alternate's URL as `loc` and every alternate as `hreflang`, which is what the PHP list types returned.

-When the settings hold a frontend URL, the backend URL (`app.url` when set, else the request's scheme and `Host`) is replaced literally inside every `<loc>`, every `xhtml:link href` and the stylesheet href, as the PHP `str_replace` did. The scheme is `https` for a TLS connection, or when the immediate peer is in `http.trusted_proxies` and its `X-Forwarded-Proto` says `https`.
+When the settings hold a frontend URL, the backend URL (`app.url` when set, else the request's scheme and `Host`) is replaced literally inside every `<loc>`, every `xhtml:link href` and the stylesheet href, as the PHP `str_replace` did. The rewrite runs only for a frontend URL `models.AllowedFrontendURL` accepts, and a `<loc>` or `href` that would not stay an `http` or `https` URL keeps its value. The scheme is `https` for a TLS connection, or when the immediate peer is in `http.trusted_proxies` and its `X-Forwarded-Proto` says `https`.

 `GET /sitemap.xsl` serves the PHP plugin's stylesheet byte for byte, `Content-Type: text/xsl; charset=UTF-8`.

@@ -103,7 +103,7 @@ A save also refuses, per row, a missing `reference` for a registry type that tak

 | Setting | Default | Meaning |
 |---------|---------|---------|
-| `frontend_url` | empty | The public frontend URL that replaces the backend URL in the XML. |
+| `frontend_url` | empty | The public frontend URL that replaces the backend URL in the XML: empty, or an absolute `http` or `https` URL with a host and without whitespace, control characters, a double quote, a less-than sign, a greater-than sign or an ampersand (`models.AllowedFrontendURL`). A stored value outside this form is not applied and the sitemap keeps the backend URL. |
 | `frontend_default_locale` | empty | The locale whose URLs carry no prefix, passed to registry types; empty leaves it to the type. |
 | `updates_manifest_path` | empty | A JSON file mapping a URL path to an ISO 8601 date, used as `lastmod` for items without their own date. |
 | `enable_priority_tiers` | on | Tier priorities replace item priorities. |
@@ -192,6 +192,7 @@ The WinterCMS forms are adapted to the admin's YAML:
 | `models.ItemTypeURL` | The built-in item type `url`. |
 | `models.Text`, `models.DecodeText` | A text field's value; the tolerant decoding of one stored text value. |
 | `models.Settings`, `models.DefaultSettings` | The settings row and its PHP defaults; `Settings.Priority` reads a tier value. |
+| `models.AllowedFrontendURL` | Whether a value may be the `frontend_url` setting: empty, or an absolute `http`/`https` URL with a host and without whitespace, control characters, `"`, `<`, `>` or `&`. |
 | `models.PriorityHomepage`, `models.PriorityServices`, `models.PriorityBlog`, `models.PriorityCategories`, `models.PriorityLegal`, `models.PriorityDefaults` | The tier setting keys and their defaults. |
 | `controllers.DefinitionsControllerID`, `controllers.PermissionAccessDefinitions` | `golem15.sitemap.definitions` and `golem15.sitemap.access_definitions`. |
 | `classes.Generator` | Writes a definition's sitemap; `Generate`, `Stylesheet`. |
@@ -200,7 +201,7 @@ The WinterCMS forms are adapted to the admin's YAML:
 | `classes.EscapeText`, `classes.EscapeAttr` | libxml's text and attribute escaping. |
 | `classes.URLTo`, `classes.BuildURL`, `classes.URLPath` | Ports of Winter `Url::to`, `Url::buildUrl` and `parse_url(..., PHP_URL_PATH)`. |
 | `classes.AllowedItemURL` | Whether a url item's address may be stored: a path or an absolute `http`/`https` URL. |
-| `classes.Rewrite`, `classes.BackendURL` | The `frontend_url` rewrite and the backend URL it replaces. |
+| `classes.Rewrite`, `classes.BackendURL` | The `frontend_url` rewrite, applied only for a value `models.AllowedFrontendURL` accepts, and the backend URL it replaces. |
 | `classes.TiersConfigKey`, `classes.TierConfig`, `classes.TierRule`, `classes.Tiers`, `classes.CompileTiers`, `classes.LoadTiers` | The tier rules. |
 | `classes.Manifest`, `classes.LoadManifest`, `classes.ManifestMaxBytes` | The updates manifest. |
 | `classes.BeforeAddItem`, `classes.AddItem`, `classes.ItemInfo` | The hooks. |
--- a/classes/rewrite.go
+++ b/classes/rewrite.go
@@ -3,6 +3,8 @@ package classes
 import (
 	"regexp"
 	"strings"
+
+	"git.golem15.com/golem15/sm-sitemap-plugin/models"
 )

 // The PHP routes.php patterns of the frontend rewrite.
@@ -16,30 +18,48 @@ var (
 // element, inside every xhtml:link href and in the exact stylesheet href.
 // Both URLs are right-trimmed of slashes first. An empty frontendURL (or
 // "0", which PHP reads as false) leaves xml unchanged.
+//
+// PHP inserted the stored text as it was. Three rules keep that text out of
+// the document unless it is a plain http(s) URL (T-17-15):
+//
+//   - a frontendURL that models.AllowedFrontendURL refuses leaves xml
+//     unchanged, whatever is stored in the settings row;
+//   - the frontend URL is inserted through EscapeText in loc and through
+//     EscapeAttr in the hrefs. Both are the identity for every accepted
+//     value, which keeps the PHP bytes;
+//   - a loc or xhtml:link href whose rewritten value does not start with
+//     http:// or https:// keeps its value. The generator applies the same
+//     filter before the rewrite.
 func Rewrite(xml, backendURL, frontendURL string) string {
 	if frontendURL == "" || frontendURL == "0" {
 		return xml
 	}
+	if !models.AllowedFrontendURL(frontendURL) {
+		return xml
+	}
 	frontend := strings.TrimRight(frontendURL, "/")
 	backend := strings.TrimRight(backendURL, "/")
-	replace := func(s string) string {
-		if backend == "" {
-			return s
-		}
-		return strings.ReplaceAll(s, backend, frontend)
+	if backend == "" {
+		return xml
 	}
+	frontendText, frontendAttr := EscapeText(frontend), EscapeAttr(frontend)
 	xml = locElement.ReplaceAllStringFunc(xml, func(m string) string {
 		inner := m[len("<loc>") : len(m)-len("</loc>")]
-		return "<loc>" + replace(inner) + "</loc>"
+		rewritten := strings.ReplaceAll(inner, backend, frontendText)
+		if !httpURL(rewritten) {
+			return m
+		}
+		return "<loc>" + rewritten + "</loc>"
 	})
 	xml = xhtmlLinkRef.ReplaceAllStringFunc(xml, func(m string) string {
 		sub := xhtmlLinkRef.FindStringSubmatch(m)
-		return sub[1] + replace(sub[2]) + sub[3]
+		rewritten := strings.ReplaceAll(sub[2], backend, frontendAttr)
+		if !httpURL(rewritten) {
+			return m
+		}
+		return sub[1] + rewritten + sub[3]
 	})
-	if backend != "" {
-		xml = strings.ReplaceAll(xml, `href="`+backend+`/sitemap.xsl"`, `href="`+frontend+`/sitemap.xsl"`)
-	}
-	return xml
+	return strings.ReplaceAll(xml, `href="`+backend+`/sitemap.xsl"`, `href="`+frontendAttr+`/sitemap.xsl"`)
 }

 // BackendURL is PHP rtrim(url('/'), '/') for a request whose root URL is
--- a/classes/rewrite_test.go
+++ b/classes/rewrite_test.go
@@ -1,22 +1,31 @@
 package classes_test

 import (
+	"context"
+	"strings"
 	"testing"

 	"git.golem15.com/golem15/sm-sitemap-plugin/classes"
 )

+// rewriteBackend and rewriteDoc are the backend URL and the document of the
+// rewrite tests: a stylesheet instruction, a loc, hreflang links and the
+// backend URL in places the rewrite leaves alone.
+const rewriteBackend = "https://api.ex.pl"
+
+var rewriteDoc = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
+	`<?xml-stylesheet type="text/xsl" href="https://api.ex.pl/sitemap.xsl"?>` + "\n" +
+	`<urlset><url><loc>https://api.ex.pl/a</loc><lastmod>2026</lastmod><changefreq>https://api.ex.pl</changefreq><priority>1</priority>` +
+	`<xhtml:link rel="alternate" hreflang="pl" href="https://api.ex.pl/a"/><xhtml:link rel="alternate" hreflang="en" href="https://api.ex.pl/en/a"/>` +
+	`<extra>https://api.ex.pl/x</extra></url><url><loc>https://other.pl/https://api.ex.pl</loc><changefreq>c</changefreq><priority>p</priority></url></urlset>` + "\n"
+
 // TestRewrite pins the PHP routes.php frontend_url rewrite: the backend URL
 // is replaced inside loc, inside xhtml:link href and in the exact stylesheet
 // href, nowhere else; slashes are trimmed on both; an empty or "0" frontend
 // URL and equal URLs leave the document unchanged.
 func TestRewrite(t *testing.T) {
-	const backend = "https://api.ex.pl"
-	doc := "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
-		`<?xml-stylesheet type="text/xsl" href="https://api.ex.pl/sitemap.xsl"?>` + "\n" +
-		`<urlset><url><loc>https://api.ex.pl/a</loc><lastmod>2026</lastmod><changefreq>https://api.ex.pl</changefreq><priority>1</priority>` +
-		`<xhtml:link rel="alternate" hreflang="pl" href="https://api.ex.pl/a"/><xhtml:link rel="alternate" hreflang="en" href="https://api.ex.pl/en/a"/>` +
-		`<extra>https://api.ex.pl/x</extra></url><url><loc>https://other.pl/https://api.ex.pl</loc><changefreq>c</changefreq><priority>p</priority></url></urlset>` + "\n"
+	const backend = rewriteBackend
+	doc := rewriteDoc
 	want := "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n" +
 		`<?xml-stylesheet type="text/xsl" href="https://www.ex.pl/sitemap.xsl"?>` + "\n" +
 		`<urlset><url><loc>https://www.ex.pl/a</loc><lastmod>2026</lastmod><changefreq>https://api.ex.pl</changefreq><priority>1</priority>` +
@@ -50,3 +59,73 @@ func TestRewrite(t *testing.T) {
 		t.Fatalf("other stylesheet = %q", got)
 	}
 }
+
+// The two frontend_url values of the T-17-15 reproduction: a script URL the
+// stylesheet would render as a link, and a value that closes the attribute
+// and the element it is written into.
+var frontendAttacks = []string{
+	"javascript:alert(document.domain)//",
+	`https://x"/><y a="`,
+}
+
+// TestRewriteUnsafeFrontendURL pins the T-17-15 output rule: a frontend URL
+// that models.AllowedFrontendURL refuses leaves the document byte for byte
+// as it was, so no part of it reaches loc, an hreflang href or the
+// stylesheet instruction; an accepted one still rewrites; and a loc or href
+// that the rewrite would turn into something other than an http(s) URL
+// keeps its value.
+func TestRewriteUnsafeFrontendURL(t *testing.T) {
+	refused := append([]string{
+		"https://www.ex.pl/?a=1&b=2",
+		"https://www.ex.pl <x>",
+		" https://www.ex.pl",
+		"https://www.ex.pl\n",
+		"//www.ex.pl",
+		"www.ex.pl",
+		"https:///x",
+		"data:text/html,x",
+	}, frontendAttacks...)
+	for _, frontend := range refused {
+		if got := classes.Rewrite(rewriteDoc, rewriteBackend, frontend); got != rewriteDoc {
+			t.Errorf("frontend %q changed the document:\n%s", frontend, got)
+		}
+	}
+
+	// The reproduction through the generator.
+	const root = "https://api.example.test"
+	xml, err := (&classes.Generator{Root: root}).Generate(context.Background(), urlItems("/blog/x"))
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !strings.Contains(xml, `href="https://api.example.test/sitemap.xsl"`) {
+		t.Fatalf("the generated document lacks the stylesheet instruction:\n%s", xml)
+	}
+	for _, frontend := range frontendAttacks {
+		got := classes.Rewrite(xml, classes.BackendURL(root), frontend)
+		if got != xml {
+			t.Errorf("frontend %q changed the generated sitemap:\n%s", frontend, got)
+		}
+		if !strings.Contains(got, "<loc>https://api.example.test/blog/x</loc>") {
+			t.Errorf("frontend %q: the loc is not on the backend URL:\n%s", frontend, got)
+		}
+		if lower := strings.ToLower(got); strings.Contains(lower, "javascript") || strings.Contains(lower, "<y a=") {
+			t.Errorf("frontend %q reached the sitemap:\n%s", frontend, got)
+		}
+	}
+
+	// A value the predicate accepts still rewrites all three places.
+	got := classes.Rewrite(xml, classes.BackendURL(root), "https://www.example.test")
+	if !strings.Contains(got, "<loc>https://www.example.test/blog/x</loc>") || !strings.Contains(got, `href="https://www.example.test/sitemap.xsl"`) || strings.Contains(got, "api.example.test") {
+		t.Errorf("a valid frontend URL did not rewrite the sitemap:\n%s", got)
+	}
+	if got := classes.Rewrite(xml, classes.BackendURL(root), "HTTPS://WWW.EXAMPLE.TEST"); !strings.Contains(got, "<loc>HTTPS://WWW.EXAMPLE.TEST/blog/x</loc>") {
+		t.Errorf("an upper-case frontend URL did not rewrite the loc:\n%s", got)
+	}
+
+	// The http(s) output filter after the rewrite: replacing "s://api.ex.pl"
+	// would write httphttps://www.ex.pl/a, so loc and href keep their values.
+	entry := `<url><loc>https://api.ex.pl/a</loc><xhtml:link rel="alternate" hreflang="pl" href="https://api.ex.pl/a"/></url>`
+	if got := classes.Rewrite(entry, "s://api.ex.pl", "https://www.ex.pl"); got != entry {
+		t.Errorf("a rewrite to a non-http(s) URL was written:\n%s", got)
+	}
+}
--- a/models/settings.go
+++ b/models/settings.go
@@ -1,5 +1,12 @@
 package models

+import (
+	"net/url"
+	"strings"
+	"unicode"
+	"unicode/utf8"
+)
+
 // The settings keys of the five priority tiers, in the order PHP checked
 // them. A tier rule names one of them.
 const (
@@ -54,6 +61,36 @@ func (Settings) Fillable() []string {
 // Rules is empty: the PHP settings form validated nothing.
 func (Settings) Rules() map[string]string { return map[string]string{} }

+// AllowedFrontendURL reports whether u may be the frontend_url setting: the
+// empty string, or an absolute http or https URL with a host and without
+// whitespace, control characters, a double quote, <, > or &. The PHP plugin
+// stored any text. The value is written into every loc, every hreflang link
+// and the stylesheet instruction of the sitemap, and sitemap.xsl renders
+// each loc as a link on the public origin, so a value outside this form
+// (a javascript: URL, or text that closes the attribute it is written into)
+// would reach the readers of /sitemap.xml (T-17-15).
+func AllowedFrontendURL(u string) bool {
+	if u == "" {
+		return true
+	}
+	if !utf8.ValidString(u) {
+		return false
+	}
+	if strings.ContainsFunc(u, func(r rune) bool {
+		return unicode.IsControl(r) || unicode.IsSpace(r) || r == '"' || r == '<' || r == '>' || r == '&'
+	}) {
+		return false
+	}
+	parsed, err := url.Parse(u)
+	if err != nil {
+		return false
+	}
+	if parsed.Scheme != "http" && parsed.Scheme != "https" {
+		return false
+	}
+	return parsed.Opaque == "" && parsed.Hostname() != ""
+}
+
 // DefaultSettings is the settings PHP read when nothing was saved: tiers on
 // with the default priorities.
 func DefaultSettings() Settings {
--- a/models/settings_test.go
+++ b/models/settings_test.go
@@ -0,0 +1,61 @@
+package models_test
+
+import (
+	"testing"
+
+	"git.golem15.com/golem15/sm-sitemap-plugin/models"
+)
+
+// TestAllowedFrontendURL pins the one definition of a frontend_url that may
+// be saved, imported and applied (T-17-15): empty, or an absolute http(s)
+// URL with a host and without whitespace, control characters, a double
+// quote, <, > or &.
+func TestAllowedFrontendURL(t *testing.T) {
+	for _, u := range []string{
+		"",
+		"https://www.ex.pl",
+		"https://www.ex.pl/",
+		"https://www.ex.pl///",
+		"http://localhost:3000",
+		"HTTPS://WWW.EX.PL",
+		"https://www.ex.pl/app",
+		"https://www.ex.pl:8443/base/",
+		"https://[::1]:3000",
+		// The frontend URLs of the golden inputs.
+		"https://api.example.test",
+		"https://www.example.test/",
+	} {
+		if !models.AllowedFrontendURL(u) {
+			t.Errorf("AllowedFrontendURL(%q) = false, want true", u)
+		}
+	}
+	for _, u := range []string{
+		"0",
+		"javascript:alert(document.domain)//",
+		"JavaScript://www.ex.pl/%0aalert(1)",
+		`https://x"/><y a="`,
+		"https://www.ex.pl/?a=1&b=2",
+		"https://www.ex.pl <x>",
+		" https://www.ex.pl",
+		"https://www.ex.pl\n",
+		"https://www.ex.pl\t",
+		"https://www.ex.pl ",
+		"https://www.ex.pl\x7f",
+		"https://www.ex.pl\xff",
+		"//www.ex.pl",
+		"www.ex.pl",
+		"/path",
+		"https:///x",
+		"https://:8080",
+		"https:www.ex.pl",
+		"http://",
+		"https://",
+		"https://%zz",
+		"data:text/html,x",
+		"ftp://www.ex.pl",
+	} {
+		if models.AllowedFrontendURL(u) {
+			t.Errorf("AllowedFrontendURL(%q) = true, want false", u)
+		}
+	}
+}
--- a/sitemap_smoke_test.go
+++ b/sitemap_smoke_test.go
@@ -424,6 +424,29 @@ func TestSitemapImportSmoke(t *testing.T) {
 	if err := console.ImportFile(ctx, e.gdb, bad, "", true, out); err == nil || err.Error() != "Entry 1 is missing a 'url' field" {
 		t.Fatalf("missing url: %v", err)
 	}
+
+	// T-17-15: a frontend URL written straight into the row (one stored
+	// before the save check existed) is not applied; the sitemap keeps the
+	// backend URL and none of the value reaches it.
+	for _, value := range []string{"javascript:alert(document.domain)//", `https://x"/><y a="`} {
+		if err := e.gdb.Model(&models.Settings{}).Where("id = ?", 1).Update("frontend_url", value).Error; err != nil {
+			t.Fatal(err)
+		}
+		rec := e.get(t, "/sitemap.xml")
+		body := rec.Body.String()
+		if rec.Code != http.StatusOK {
+			t.Fatalf("GET /sitemap.xml with frontend_url %q: %d %s", value, rec.Code, body)
+		}
+		if lower := strings.ToLower(body); strings.Contains(lower, "javascript") || strings.Contains(lower, "<y a=") {
+			t.Errorf("frontend_url %q reached the sitemap:\n%s", value, body)
+		}
+		if !strings.Contains(body, `href="https://api.example.test/sitemap.xsl"`) || !strings.Contains(body, "<loc>https://api.example.test/x</loc>") {
+			t.Errorf("frontend_url %q: the sitemap is not on the backend URL:\n%s", value, body)
+		}
+		if strings.Contains(body, "www.example.test") {
+			t.Errorf("frontend_url %q: the earlier frontend URL is left in the sitemap:\n%s", value, body)
+		}
+	}
 }

 var adminSeq atomic.Uint64