diff --git a/middleware/fileserver.go b/middleware/fileserver.go index 80cf6f841..a1efd945b 100644 --- a/middleware/fileserver.go +++ b/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. diff --git a/middleware/fileserver_test.go b/middleware/fileserver_test.go index 0f5b1faca..c90112383 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,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 {