mirror of
https://github.com/caddyserver/caddy.git
synced 2026-11-11 19:37:55 +00:00
rewrite: canonicalize paths after substring and regexp replacement
Like prefix/suffix stripping, substring and regexp path replacement can produce a relative path or expose dot segments that later path-scoped handlers interpret differently. Apply the existing canonicalizePath safeguard whenever these operations change the path. Addresses scan report 65568b0d767b733c. Code written by GitHub Copilot (GPT-6.1 Sol, Claude Opus 5.5); reviewed by mholt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
1 parent
fe69f5da2d
commit
fc6c202ea6
2 files changed
+58
-5
No files matched your search
@@ -83,7 +83,8 @@ type Rewrite struct {
|
||||
// to have that same escape at that position in order to match.
|
||||
StripPathSuffix string `json:"strip_path_suffix,omitempty"`
|
||||
|
||||
// Performs substring replacements on the URI.
|
||||
// Performs substring replacements on the URI. If a replacement
|
||||
// changes the path, dot segments in the result are cleaned.
|
||||
URISubstring []substrReplacer `json:"uri_substring,omitempty"`
|
||||
|
||||
// Performs regular expression replacements on the URI path.
|
||||
@@ -581,9 +582,13 @@ func (rep substrReplacer) do(r *http.Request, repl *caddy.Replacer) {
|
||||
|
||||
mergeSlashes := !strings.Contains(rep.Find, "//")
|
||||
|
||||
oldPath := r.URL.Path
|
||||
changePath(r, func(pathOrRawPath string) string {
|
||||
return strings.Replace(caddyhttp.CleanPath(pathOrRawPath, mergeSlashes), find, replace, lim)
|
||||
})
|
||||
if r.URL.Path != oldPath {
|
||||
canonicalizePath(r)
|
||||
}
|
||||
|
||||
r.URL.RawQuery = strings.Replace(r.URL.RawQuery, find, replace, lim)
|
||||
}
|
||||
@@ -605,9 +610,13 @@ func (rep regexReplacer) do(r *http.Request, repl *caddy.Replacer) {
|
||||
return
|
||||
}
|
||||
replace := repl.ReplaceAll(rep.Replace, "")
|
||||
oldPath := r.URL.Path
|
||||
changePath(r, func(pathOrRawPath string) string {
|
||||
return rep.re.ReplaceAllString(pathOrRawPath, replace)
|
||||
})
|
||||
if r.URL.Path != oldPath {
|
||||
canonicalizePath(r)
|
||||
}
|
||||
}
|
||||
|
||||
func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
|
||||
@@ -625,11 +634,10 @@ func changePath(req *http.Request, newVal func(pathOrRawPath string) string) {
|
||||
}
|
||||
}
|
||||
|
||||
// canonicalizePath anchors and cleans a stripped origin-form path while
|
||||
// canonicalizePath anchors and cleans a modified origin-form path while
|
||||
// preserving an alternate RawPath encoding when it still represents the
|
||||
// canonical Path. It is applied after prefix or suffix removal because stripping can
|
||||
// expose a relative path or dot segments that downstream handlers interpret
|
||||
// differently.
|
||||
// canonical Path. Removing or replacing path components can expose a relative
|
||||
// path or dot segments that downstream handlers interpret differently.
|
||||
func canonicalizePath(req *http.Request) {
|
||||
p := req.URL.Path
|
||||
if !strings.HasPrefix(p, "/") {
|
||||
|
||||
@@ -15,6 +15,7 @@
|
||||
package rewrite
|
||||
|
||||
import (
|
||||
"context"
|
||||
"net/http"
|
||||
"reflect"
|
||||
"regexp"
|
||||
@@ -22,6 +23,7 @@ import (
|
||||
"testing"
|
||||
|
||||
"github.com/caddyserver/caddy/v2"
|
||||
"github.com/caddyserver/caddy/v2/modules/caddyhttp"
|
||||
)
|
||||
|
||||
func TestRewrite(t *testing.T) {
|
||||
@@ -828,6 +830,49 @@ func TestStripPathPrefixCanonicalizesRemainder(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestPathReplacementsCanonicalizeRemainder(t *testing.T) {
|
||||
for _, rule := range []struct {
|
||||
name string
|
||||
rule Rewrite
|
||||
}{
|
||||
{"substring", Rewrite{URISubstring: []substrReplacer{{Find: "/mount"}}}},
|
||||
{"regexp", Rewrite{PathRegexp: []*regexReplacer{{Find: "^/mount", re: regexp.MustCompile("^/mount")}}}},
|
||||
} {
|
||||
t.Run(rule.name, func(t *testing.T) {
|
||||
for _, tc := range []struct {
|
||||
target, path, rawPath string
|
||||
}{
|
||||
{"/mount../secret", "/secret", ""},
|
||||
{"/mount..%2fsecret", "/secret", ""},
|
||||
{"/mount%2e%2e/secret", "/secret", ""},
|
||||
{"/mount", "/", ""},
|
||||
{"/mountain", "/ain", ""},
|
||||
{"/mount/public", "/public", ""},
|
||||
{"/mount/a%2Fb", "/a/b", "/a%2Fb"},
|
||||
{"/mount/%252e%252e/secret", "/%2e%2e/secret", ""},
|
||||
{"/mount/%ff", "/\xff", "/%ff"},
|
||||
{"/unmatched", "/unmatched", ""},
|
||||
} {
|
||||
t.Run(tc.target, func(t *testing.T) {
|
||||
r := newRequest(t, http.MethodGet, tc.target+"?keep=yes")
|
||||
repl := caddy.NewReplacer()
|
||||
r = r.WithContext(context.WithValue(r.Context(), caddy.ReplacerCtxKey, repl))
|
||||
rule.rule.Rewrite(r, repl)
|
||||
if r.URL.Path != tc.path || r.URL.RawPath != tc.rawPath {
|
||||
t.Errorf("Path/RawPath = %q/%q; want %q/%q", r.URL.Path, r.URL.RawPath, tc.path, tc.rawPath)
|
||||
}
|
||||
if r.URL.RawQuery != "keep=yes" || r.RequestURI != r.URL.EscapedPath()+"?keep=yes" {
|
||||
t.Errorf("inconsistent request target: %q", r.RequestURI)
|
||||
}
|
||||
if tc.path == "/secret" && !(caddyhttp.MatchPath{"/secret*"}).Match(r) {
|
||||
t.Error("rewritten secret does not match downstream path restriction")
|
||||
}
|
||||
})
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func newRequest(t *testing.T, method, uri string) *http.Request {
|
||||
req, err := http.NewRequest(method, uri, nil)
|
||||
if err != nil {
|
||||
|
||||
Reference in new issue
Block a user