diff --git a/resources/index.html b/resources/index.html new file mode 100644 index 00000000..6c70bcfe --- /dev/null +++ b/resources/index.html @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/router.go b/router.go index 500e68dc..fc7c0e33 100644 --- a/router.go +++ b/router.go @@ -445,6 +445,10 @@ func (r *Router) Options(uri string, handler Handler) *Route { // // If no file is given in the url, or if the given file is a directory, the handler will // send the "index.html" file if it exists. +// +// As a precaution, all requests with a path containing a path segment in the form of ".", ".." are rejected with +// `http.StatusNotFound`. This ensures clients cannot access files outside of the given filesystem base directory. +// Paths containing "\" or "//" are also rejected. func (r *Router) Static(fs fs.StatFS, uri string, download bool) *Route { return r.registerRoute([]string{http.MethodGet}, uri+"{resource:.*}", staticHandler(fs, download)) } diff --git a/static.go b/static.go index 107a0002..0e7644a3 100644 --- a/static.go +++ b/static.go @@ -2,18 +2,25 @@ package goyave import ( "io/fs" + "net/http" + "path/filepath" "strings" + "github.com/samber/lo" "goyave.dev/goyave/v5/util/fsutil" ) func staticHandler(fs fs.StatFS, download bool) Handler { return func(response *Response, r *Request) { file := r.RouteParams["resource"] + if !checkStaticPath(file) { + response.Status(http.StatusNotFound) + return + } path := cleanStaticPath(fs, file) if download { - response.Download(fs, path, file[strings.LastIndex(file, "/")+1:]) + response.Download(fs, path, path[lo.Clamp(strings.LastIndex(file, "/"), 0, len(path)):]) return } response.File(fs, path) @@ -32,5 +39,24 @@ func cleanStaticPath(fs fs.StatFS, file string) string { } path += "index.html" } - return path + return filepath.Clean(path) +} + +func checkStaticPath(path string) bool { + if !strings.HasPrefix(path, "/") && len(path) > 0 { // Force leading slash if the path is not empty + return false + } + if strings.Contains(path, "\\") || strings.Contains(path, "//") { + return false + } + for _, ent := range strings.FieldsFunc(path, isSlash) { + if ent == "." || ent == ".." || ent == "" { + return false + } + } + return true +} + +func isSlash(r rune) bool { + return r == '/' } diff --git a/static_test.go b/static_test.go index 48956926..a66f1358 100644 --- a/static_test.go +++ b/static_test.go @@ -5,6 +5,7 @@ import ( "io/fs" "net/http" "net/http/httptest" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -70,6 +71,74 @@ func TestStaticHandler(t *testing.T) { assert.Equal(t, http.StatusNotFound, response.GetStatus()) }, }, + { + uri: "custom_config.json", // Force leading slash if the path is not empty + directory: "resources", + download: false, + expected: func(t *testing.T, response *Response, _ *http.Response, _ []byte) { + assert.Equal(t, http.StatusNotFound, response.GetStatus()) + }, + }, + { + uri: "index.html", // Force leading slash if the path is not empty + directory: "resources", + download: false, + expected: func(t *testing.T, response *Response, _ *http.Response, _ []byte) { + assert.Equal(t, http.StatusNotFound, response.GetStatus()) + }, + }, + { + uri: "", + directory: "resources", + download: false, + expected: func(t *testing.T, response *Response, result *http.Response, body []byte) { + assert.Equal(t, http.StatusOK, response.GetStatus()) + assert.Equal(t, "text/html; charset=utf-8", result.Header.Get("Content-Type")) + assert.Equal(t, "", string(body)) + }, + }, + { + uri: "", // Download index.html + directory: "resources", + download: true, + expected: func(t *testing.T, response *Response, result *http.Response, body []byte) { + assert.Equal(t, http.StatusOK, response.GetStatus()) + assert.Equal(t, "text/html; charset=utf-8", result.Header.Get("Content-Type")) + assert.Equal(t, "attachment; filename=\"index.html\"", result.Header.Get("Content-Disposition")) + assert.Equal(t, "", string(body)) + }, + }, + { + uri: "/", + directory: "resources", + download: false, + expected: func(t *testing.T, response *Response, result *http.Response, body []byte) { + assert.Equal(t, http.StatusOK, response.GetStatus()) + assert.Equal(t, "text/html; charset=utf-8", result.Header.Get("Content-Type")) + assert.Equal(t, "", string(body)) + }, + }, + { + uri: "/index.html", + directory: "resources", + download: false, + expected: func(t *testing.T, response *Response, result *http.Response, body []byte) { + assert.Equal(t, http.StatusOK, response.GetStatus()) + assert.Equal(t, "text/html; charset=utf-8", result.Header.Get("Content-Type")) + assert.Equal(t, "", string(body)) + }, + }, + { + uri: "/", // Download index.html + directory: "resources", + download: true, + expected: func(t *testing.T, response *Response, result *http.Response, body []byte) { + assert.Equal(t, http.StatusOK, response.GetStatus()) + assert.Equal(t, "text/html; charset=utf-8", result.Header.Get("Content-Type")) + assert.Equal(t, "attachment; filename=\"index.html\"", result.Header.Get("Content-Disposition")) + assert.Equal(t, "", string(body)) + }, + }, { uri: "/custom_config.json", directory: "resources", @@ -95,12 +164,12 @@ func TestStaticHandler(t *testing.T) { } for _, c := range cases { - t.Run(c.uri, func(t *testing.T) { + t.Run(strings.ReplaceAll(c.uri, "/", "_"), func(t *testing.T) { cfg := config.LoadDefault() srv, err := New(Options{Config: cfg}) require.NoError(t, err) - request := NewRequest(httptest.NewRequest(http.MethodGet, c.uri, nil)) + request := NewRequest(httptest.NewRequest(http.MethodGet, "/static"+c.uri, nil)) request.RouteParams = map[string]string{"resource": c.uri} recorder := httptest.NewRecorder() @@ -119,3 +188,49 @@ func TestStaticHandler(t *testing.T) { }) } } + +func TestStaticHandlerBounds(t *testing.T) { + // This test ensures a client cannot request a file outside of + // the bounds of the set directory (using . or ..) + cases := []string{ + "/resources/.", + "/resources/./", + "/resources/..", + "/resources/../", + "/resources/img/..", + "/resources/img/../custom_config.json", + "/resources/img/./logo/goyave_16.png", + "/resources/img//logo/goyave_16.png", + "/resources/\\", + "/resources/img\\logo\\goyave_16.png", + "/resources/../README.md", + "/resources//custom_config.json", + "/.", + "/..", + ".", + "..", + } + + for _, c := range cases { + t.Run(strings.ReplaceAll(c, "/", "_"), func(t *testing.T) { + cfg := config.LoadDefault() + srv, err := New(Options{Config: cfg}) + require.NoError(t, err) + + request := NewRequest(httptest.NewRequest(http.MethodGet, "/static"+c, nil)) + request.RouteParams = map[string]string{"resource": c} + + recorder := httptest.NewRecorder() + response := NewResponse(srv, request, recorder) + + require.NoError(t, err) + handler := staticHandler(&osfs.FS{}, false) + handler(response, request) + + result := recorder.Result() + assert.NoError(t, result.Body.Close()) + require.NoError(t, err) + assert.Equal(t, http.StatusNotFound, response.GetStatus()) + }) + } +}