From 008ad398cec59c7af600c501ae34a76b5ca1fe0a Mon Sep 17 00:00:00 2001
From: Abiola Ibrahim <abiola89@gmail.com>
Date: Sat, 12 Mar 2016 17:47:53 +0100
Subject: [PATCH] Hopefully, this is the final nail on the coffin.

---
 middleware/fileserver.go      | 30 ++++++++++--------------------
 middleware/fileserver_test.go | 25 +++++++++++++++----------
 2 files changed, 25 insertions(+), 30 deletions(-)

diff --git a/middleware/fileserver.go b/middleware/fileserver.go
index 6cdb0ff5..a1efd945 100644
--- a/middleware/fileserver.go
+++ b/middleware/fileserver.go
@@ -122,7 +122,7 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st
 	}
 
 	// If file is on hide list.
-	if fh.isHidden(d.Name()) {
+	if fh.isHidden(d) {
 		return http.StatusNotFound, nil
 	}
 
@@ -133,28 +133,18 @@ func (fh *fileHandler) serveFile(w http.ResponseWriter, r *http.Request, name st
 	return http.StatusOK, nil
 }
 
-// isHidden checks if file with name is on hide list.
-func (fh fileHandler) isHidden(name string) bool {
-	// Clean up on Windows.
-	// Remove trailing dots and trim whitespaces.
-	if runtimeGoos == "windows" {
-		name = strings.TrimSpace(name)
-		for strings.HasSuffix(name, ".") {
-			name = name[:len(name)-1]
-			name = strings.TrimSpace(name)
-		}
-	}
+// 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 {
-		// 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(name, filepath.Base(hiddenPath)) {
-			return true
+		// 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
diff --git a/middleware/fileserver_test.go b/middleware/fileserver_test.go
index ba2f23ba..c9011238 100644
--- a/middleware/fileserver_test.go
+++ b/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,54 +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 11 - attempt to bypass hidden file
+		// Test 12 - attempt to bypass hidden file
 		{
 			url:            "https://foo/dir/hidden.html%20",
 			expectedStatus: http.StatusNotFound,
 		},
-		// Test 12 - attempt to bypass hidden file
+		// Test 13 - attempt to bypass hidden file
 		{
 			url:            "https://foo/dir/hidden.html.",
 			expectedStatus: http.StatusNotFound,
 		},
-		// Test 13 - attempt to bypass hidden file
+		// Test 14 - attempt to bypass hidden file
 		{
 			url:            "https://foo/dir/hidden.html.%20",
 			expectedStatus: http.StatusNotFound,
 		},
-		// Test 14 - attempt to bypass hidden file
+		// 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 {