diff --git a/caddyhttp/staticfiles/fileserver.go b/caddyhttp/staticfiles/fileserver.go index 2b38212e..91fb1a7f 100644 --- a/caddyhttp/staticfiles/fileserver.go +++ b/caddyhttp/staticfiles/fileserver.go @@ -107,6 +107,10 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request) (int, err if d.IsDir() { // ensure there is a trailing slash if urlCopy.Path[len(urlCopy.Path)-1] != '/' { + for strings.HasPrefix(urlCopy.Path, "//") { + // prevent path-based open redirects + urlCopy.Path = strings.TrimPrefix(urlCopy.Path, "/") + } urlCopy.Path += "/" http.Redirect(w, r, urlCopy.String(), http.StatusMovedPermanently) return http.StatusMovedPermanently, nil @@ -131,6 +135,10 @@ func (fs FileServer) serveFile(w http.ResponseWriter, r *http.Request) (int, err } if redir { + for strings.HasPrefix(urlCopy.Path, "//") { + // prevent path-based open redirects + urlCopy.Path = strings.TrimPrefix(urlCopy.Path, "/") + } http.Redirect(w, r, urlCopy.String(), http.StatusMovedPermanently) return http.StatusMovedPermanently, nil } diff --git a/caddyhttp/staticfiles/fileserver_test.go b/caddyhttp/staticfiles/fileserver_test.go index 9cce7705..80d8f1a4 100644 --- a/caddyhttp/staticfiles/fileserver_test.go +++ b/caddyhttp/staticfiles/fileserver_test.go @@ -77,9 +77,9 @@ func TestServeHTTP(t *testing.T) { { url: "https://foo/dirwithindex/", expectedStatus: http.StatusOK, - expectedBodyContent: testFiles[webrootDirwithindexIndeHTML], + expectedBodyContent: testFiles[webrootDirwithindexIndexHTML], expectedEtag: `"2n9cw"`, - expectedContentLength: strconv.Itoa(len(testFiles[webrootDirwithindexIndeHTML])), + expectedContentLength: strconv.Itoa(len(testFiles[webrootDirwithindexIndexHTML])), }, // Test 4 - access folder with index file without trailing slash { @@ -235,16 +235,38 @@ func TestServeHTTP(t *testing.T) { expectedBodyContent: movedPermanently, }, { + // Test 27 - Check etag url: "https://foo/notindex.html", expectedStatus: http.StatusOK, expectedBodyContent: testFiles[webrootNotIndexHTML], expectedEtag: `"2n9cm"`, expectedContentLength: strconv.Itoa(len(testFiles[webrootNotIndexHTML])), }, + { + // Test 28 - Prevent path-based open redirects (directory) + url: "https://foo//example.com%2f..", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/example.com/../", + expectedBodyContent: movedPermanently, + }, + { + // Test 29 - Prevent path-based open redirects (file) + url: "https://foo//example.com%2f../dirwithindex/index.html", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/example.com/../dirwithindex/", + expectedBodyContent: movedPermanently, + }, + { + // Test 29 - Prevent path-based open redirects (extra leading slashes) + url: "https://foo///example.com%2f..", + expectedStatus: http.StatusMovedPermanently, + expectedLocation: "https://foo/example.com/../", + expectedBodyContent: movedPermanently, + }, } for i, test := range tests { - // set up response writer and rewuest + // set up response writer and request responseRecorder := httptest.NewRecorder() request, err := http.NewRequest("GET", test.url, nil) if err != nil { @@ -518,7 +540,7 @@ var ( webrootNotIndexHTML = filepath.Join(webrootName, "notindex.html") webrootDirFile2HTML = filepath.Join(webrootName, "dir", "file2.html") webrootDirHiddenHTML = filepath.Join(webrootName, "dir", "hidden.html") - webrootDirwithindexIndeHTML = filepath.Join(webrootName, "dirwithindex", "index.html") + webrootDirwithindexIndexHTML = filepath.Join(webrootName, "dirwithindex", "index.html") webrootSubGzippedHTML = filepath.Join(webrootName, "sub", "gzipped.html") webrootSubGzippedHTMLGz = filepath.Join(webrootName, "sub", "gzipped.html.gz") webrootSubGzippedHTMLBr = filepath.Join(webrootName, "sub", "gzipped.html.br") @@ -544,7 +566,7 @@ var testFiles = map[string]string{ webrootFile1HTML: "