Merge pull request #672 from abiosoft/master

Hide only the currently used Caddyfile

middleware/fileserver.go

@@ -4,6 +4,7 @@ import (
 	"net/http"
 	"os"
 	"path"
+	"path/filepath"
 	"strings"
 )
 
@@ -49,6 +50,13 @@ func (fh *fileHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) (int, e
 // serveFile writes the specified file to the HTTP response.
 // name is '/'-separated, not filepath.Separator.
 func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name string) (int, error) {
+	// Prevent absolute path access on Windows.
+	// TODO remove when stdlib http.Dir fixes this.
+	if runtimeGoos == "windows" {
+		if filepath.IsAbs(name[1:]) {
+			return http.StatusNotFound, nil
+		}
+	}
 	f, err := fh.root.Open(name)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -113,18 +121,9 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st
 		return http.StatusNotFound, nil
 	}
 
-	// If the file is supposed to be hidden, return a 404
-	// (TODO: If the slice gets large, a set may be faster)
-	for _, hiddenPath := range fh.hide {
-		// Case-insensitive file systems may have loaded "CaddyFile" when
-		// we think we got "Caddyfile", which poses a security risk if we
-		// aren't careful here: case-insensitive comparison is required!
-		// TODO: This matches file NAME only, regardless of path. In other
-		// words, trying to serve another file with the same name as the
-		// active config file will result in a 404 when it shouldn't.
-		if strings.EqualFold(d.Name(), path.Base(hiddenPath)) {
-			return http.StatusNotFound, nil
-		}
+	// If file is on hide list.
+	if fh.isHidden(d) {
+		return http.StatusNotFound, nil
 	}
 
 	// Note: Errors generated by ServeContent are written immediately
@@ -134,6 +133,23 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st
 	return http.StatusOK, nil
 }
 
+// isHidden checks if file with FileInfo d is on hide list.
+func (fh fileHandler) isHidden(d os.FileInfo) bool {
+	// If the file is supposed to be hidden, return a 404
+	// (TODO: If the slice gets large, a set may be faster)
+	for _, hiddenPath := range fh.hide {
+		// Check if the served file is exactly the hidden file.
+		if hFile, err := fh.root.Open(hiddenPath); err == nil {
+			fs, _ := hFile.Stat()
+			hFile.Close()
+			if os.SameFile(d, fs) {
+				return true
+			}
+		}
+	}
+	return false
+}
+
 // redirect is taken from http.localRedirect of the std lib. It
 // sends an HTTP redirect to the client but will preserve the
 // query string for the new path.

middleware/fileserver_test.go

@@ -35,7 +35,7 @@ func TestServeHTTP(t *testing.T) {
 	beforeServeHTTPTest(t)
 	defer afterServeHTTPTest(t)
 
-	fileserver := FileServer(http.Dir(testDir), []string{"hidden.html"})
+	fileserver := FileServer(http.Dir(testDir), []string{"dir/hidden.html"})
 
 	movedPermanently := "Moved Permanently"
 
@@ -84,34 +84,59 @@ func TestServeHTTP(t *testing.T) {
 			expectedStatus:      http.StatusMovedPermanently,
 			expectedBodyContent: movedPermanently,
 		},
-		// Test 6 - access file with trailing slash
+		// Test 7 - access file with trailing slash
 		{
 			url:                 "https://foo/file1.html/",
 			expectedStatus:      http.StatusMovedPermanently,
 			expectedBodyContent: movedPermanently,
 		},
-		// Test 7 - access not existing path
+		// Test 8 - access not existing path
 		{
 			url:            "https://foo/not_existing",
 			expectedStatus: http.StatusNotFound,
 		},
-		// Test 8 - access a file, marked as hidden
+		// Test 9 - access a file, marked as hidden
 		{
 			url:            "https://foo/dir/hidden.html",
 			expectedStatus: http.StatusNotFound,
 		},
-		// Test 9 - access a index file directly
+		// Test 10 - access a index file directly
 		{
 			url:                 "https://foo/dirwithindex/index.html",
 			expectedStatus:      http.StatusOK,
 			expectedBodyContent: testFiles[filepath.Join("dirwithindex", "index.html")],
 		},
-		// Test 10 - send a request with query params
+		// Test 11 - send a request with query params
 		{
 			url:                 "https://foo/dir?param1=val",
 			expectedStatus:      http.StatusMovedPermanently,
 			expectedBodyContent: movedPermanently,
 		},
+		// Test 12 - attempt to bypass hidden file
+		{
+			url:            "https://foo/dir/hidden.html%20",
+			expectedStatus: http.StatusNotFound,
+		},
+		// Test 13 - attempt to bypass hidden file
+		{
+			url:            "https://foo/dir/hidden.html.",
+			expectedStatus: http.StatusNotFound,
+		},
+		// Test 14 - attempt to bypass hidden file
+		{
+			url:            "https://foo/dir/hidden.html.%20",
+			expectedStatus: http.StatusNotFound,
+		},
+		// Test 15 - attempt to bypass hidden file
+		{
+			url:            "https://foo/dir/hidden.html%20.",
+			expectedStatus: http.StatusNotFound,
+		},
+		// Test 16 - serve another file with same name as hidden file.
+		{
+			url:            "https://foo/hidden.html",
+			expectedStatus: http.StatusNotFound,
+		},
 	}
 
 	for i, test := range tests {