From 33c70f418f780f8e9524c73fbf4bbdbdbb9d7500 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Thu, 16 Sep 2021 23:40:31 +0300 Subject: fileserver: properly handle escaped/non-ascii paths (#4332) * fileserver: properly handle escaped/non-ascii paths * fileserver: tests: accommodate Windows hate of colons in files names --- modules/caddyhttp/caddyhttp.go | 2 + modules/caddyhttp/fileserver/browsetplcontext.go | 7 ++-- modules/caddyhttp/fileserver/matcher_test.go | 43 ++++++++++++++++++++++ modules/caddyhttp/fileserver/staticfiles.go | 11 ++++++ .../fileserver/testdata/%D9%85%D9%84%D9%81.txt | 1 + .../testdata/\331\205\331\204\331\201.txt" | 1 + 6 files changed, 62 insertions(+), 3 deletions(-) create mode 100644 modules/caddyhttp/fileserver/testdata/%D9%85%D9%84%D9%81.txt create mode 100644 "modules/caddyhttp/fileserver/testdata/\331\205\331\204\331\201.txt" (limited to 'modules/caddyhttp') diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go index 784b2b9..1404c79 100644 --- a/modules/caddyhttp/caddyhttp.go +++ b/modules/caddyhttp/caddyhttp.go @@ -20,6 +20,7 @@ import ( "io" "net" "net/http" + "net/url" "path/filepath" "strconv" "strings" @@ -227,6 +228,7 @@ func StatusCodeMatches(actual, configured int) bool { // never be outside of root. The resulting path can be used // with the local file system. func SanitizedPathJoin(root, reqPath string) string { + reqPath, _ = url.PathUnescape(reqPath) if root == "" { root = "." } diff --git a/modules/caddyhttp/fileserver/browsetplcontext.go b/modules/caddyhttp/fileserver/browsetplcontext.go index d420c48..6aa7501 100644 --- a/modules/caddyhttp/fileserver/browsetplcontext.go +++ b/modules/caddyhttp/fileserver/browsetplcontext.go @@ -42,15 +42,16 @@ func (fsrv *FileServer) directoryListing(files []os.FileInfo, canGoUp bool, root isDir := f.IsDir() || isSymlinkTargetDir(f, root, urlPath) + u := url.URL{Path: url.PathEscape(name)} + + // add the slash after the escape of path to avoid escaping the slash as well if isDir { - name += "/" + u.Path += "/" dirCount++ } else { fileCount++ } - u := url.URL{Path: "./" + name} // prepend with "./" to fix paths with ':' in the name - fileInfos = append(fileInfos, fileInfo{ IsDir: isDir, IsSymlink: isSymlink(f), diff --git a/modules/caddyhttp/fileserver/matcher_test.go b/modules/caddyhttp/fileserver/matcher_test.go index 746a5d7..5b6078a 100644 --- a/modules/caddyhttp/fileserver/matcher_test.go +++ b/modules/caddyhttp/fileserver/matcher_test.go @@ -17,12 +17,31 @@ package fileserver import ( "net/http" "net/url" + "os" + "runtime" "testing" "github.com/caddyserver/caddy/v2/modules/caddyhttp" ) func TestFileMatcher(t *testing.T) { + + // Windows doesn't like colons in files names + isWindows := runtime.GOOS == "windows" + if !isWindows { + filename := "with:in-name.txt" + f, err := os.Create("./testdata/" + filename) + if err != nil { + t.Fail() + return + } + t.Cleanup(func() { + os.Remove("./testdata/" + filename) + }) + f.WriteString(filename) + f.Close() + } + for i, tc := range []struct { path string expectedPath string @@ -63,6 +82,30 @@ func TestFileMatcher(t *testing.T) { path: "/missingfile.php", matched: false, }, + { + path: "ملف.txt", // the path file name is not escaped + expectedPath: "ملف.txt", + expectedType: "file", + matched: true, + }, + { + path: url.PathEscape("ملف.txt"), // singly-escaped path + expectedPath: "ملف.txt", + expectedType: "file", + matched: true, + }, + { + path: url.PathEscape(url.PathEscape("ملف.txt")), // doubly-escaped path + expectedPath: "%D9%85%D9%84%D9%81.txt", + expectedType: "file", + matched: true, + }, + { + path: "./with:in-name.txt", // browsers send the request with the path as such + expectedPath: "with:in-name.txt", + expectedType: "file", + matched: !isWindows, + }, } { m := &MatchFile{ Root: "./testdata", diff --git a/modules/caddyhttp/fileserver/staticfiles.go b/modules/caddyhttp/fileserver/staticfiles.go index 592b317..3e096e1 100644 --- a/modules/caddyhttp/fileserver/staticfiles.go +++ b/modules/caddyhttp/fileserver/staticfiles.go @@ -19,6 +19,7 @@ import ( weakrand "math/rand" "mime" "net/http" + "net/url" "os" "path" "path/filepath" @@ -165,6 +166,16 @@ func (fsrv *FileServer) ServeHTTP(w http.ResponseWriter, r *http.Request, next c filesToHide := fsrv.transformHidePaths(repl) root := repl.ReplaceAll(fsrv.Root, ".") + // PathUnescape returns an error if the escapes aren't well-formed, + // meaning the count % matches the RFC. Return early if the escape is + // improper. + if _, err := url.PathUnescape(r.URL.Path); err != nil { + fsrv.logger.Debug("improper path escape", + zap.String("site_root", root), + zap.String("request_path", r.URL.Path), + zap.Error(err)) + return err + } filename := caddyhttp.SanitizedPathJoin(root, r.URL.Path) fsrv.logger.Debug("sanitized path join", diff --git a/modules/caddyhttp/fileserver/testdata/%D9%85%D9%84%D9%81.txt b/modules/caddyhttp/fileserver/testdata/%D9%85%D9%84%D9%81.txt new file mode 100644 index 0000000..0f4bf1a --- /dev/null +++ b/modules/caddyhttp/fileserver/testdata/%D9%85%D9%84%D9%81.txt @@ -0,0 +1 @@ +%D9%85%D9%84%D9%81.txt \ No newline at end of file diff --git "a/modules/caddyhttp/fileserver/testdata/\331\205\331\204\331\201.txt" "b/modules/caddyhttp/fileserver/testdata/\331\205\331\204\331\201.txt" new file mode 100644 index 0000000..9185828 --- /dev/null +++ "b/modules/caddyhttp/fileserver/testdata/\331\205\331\204\331\201.txt" @@ -0,0 +1 @@ +ملف.txt \ No newline at end of file -- cgit v1.2.3