From 33c70f418f780f8e9524c73fbf4bbdbdbb9d7500 Mon Sep 17 00:00:00 2001
From: Mohammed Al Sahaf <msaa1990@gmail.com>
Date: Thu, 16 Sep 2021 23:40:31 +0300
Subject: [PATCH] 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 +
 .../caddyhttp/fileserver/browsetplcontext.go  |  7 +--
 modules/caddyhttp/fileserver/matcher_test.go  | 43 +++++++++++++++++++
 modules/caddyhttp/fileserver/staticfiles.go   | 11 +++++
 .../testdata/%D9%85%D9%84%D9%81.txt           |  1 +
 modules/caddyhttp/fileserver/testdata/ملف.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/ملف.txt

diff --git a/modules/caddyhttp/caddyhttp.go b/modules/caddyhttp/caddyhttp.go
index 784b2b90..1404c796 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 d420c489..6aa75013 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 746a5d78..5b6078ad 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 592b3171..3e096e1f 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 00000000..0f4bf1a9
--- /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/ملف.txt b/modules/caddyhttp/fileserver/testdata/ملف.txt
new file mode 100644
index 00000000..91858286
--- /dev/null
+++ b/modules/caddyhttp/fileserver/testdata/ملف.txt
@@ -0,0 +1 @@
+ملف.txt
\ No newline at end of file