From 34dfb49b719c948e709a4639b4af2c5cb73b3887 Mon Sep 17 00:00:00 2001 From: Ramires Viana <59319979+ramiresviana@users.noreply.github.com> Date: Mon, 20 Jul 2020 17:37:38 +0000 Subject: [PATCH 1/5] fix: path separator inconsistency on rename --- http/resource.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/http/resource.go b/http/resource.go index 20376968..7ddbbd07 100644 --- a/http/resource.go +++ b/http/resource.go @@ -168,7 +168,7 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, break } new := fmt.Sprintf("%s(%d)%s", base, counter, ext) - dst = filepath.Join(dir, new) + dst = filepath.ToSlash(dir) + new counter++ } } From 727c63b98e2964d0960d25914c296570f6c79478 Mon Sep 17 00:00:00 2001 From: Ramires Viana <59319979+ramiresviana@users.noreply.github.com> Date: Mon, 20 Jul 2020 17:38:43 +0000 Subject: [PATCH 2/5] fix: parent verification on copy --- errors/errors.go | 1 + http/resource.go | 58 ++++++++++++++++++++++++++++++++++-------------- 2 files changed, 42 insertions(+), 17 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 592cabe3..c79e3783 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -16,4 +16,5 @@ var ( ErrInvalidAuthMethod = errors.New("invalid auth method") ErrPermissionDenied = errors.New("permission denied") ErrInvalidRequestParams = errors.New("invalid request params") + ErrSourceIsParent = errors.New("source is parent") ) diff --git a/http/resource.go b/http/resource.go index 7ddbbd07..55a24a52 100644 --- a/http/resource.go +++ b/http/resource.go @@ -10,6 +10,8 @@ import ( "path/filepath" "strings" + "github.com/spf13/afero" + "github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/files" "github.com/filebrowser/filebrowser/v2/fileutils" @@ -139,38 +141,25 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, dst := r.URL.Query().Get("destination") action := r.URL.Query().Get("action") dst, err := url.QueryUnescape(dst) - if err != nil { return errToStatus(err), err } - if dst == "/" || src == "/" { return http.StatusForbidden, nil } + if err = checkParent(src, dst); err != nil { + return http.StatusBadRequest, err + } override := r.URL.Query().Get("override") == "true" rename := r.URL.Query().Get("rename") == "true" - if !override && !rename { if _, err = d.user.Fs.Stat(dst); err == nil { return http.StatusConflict, nil } } - if rename { - counter := 1 - dir, name := filepath.Split(dst) - ext := filepath.Ext(name) - base := strings.TrimSuffix(name, ext) - - for { - if _, err = d.user.Fs.Stat(dst); err != nil { - break - } - new := fmt.Sprintf("%s(%d)%s", base, counter, ext) - dst = filepath.ToSlash(dir) + new - counter++ - } + dst = addVersionSuffix(dst, d.user.Fs) } err = d.RunHook(func() error { @@ -180,11 +169,14 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, if !d.user.Perm.Create { return errors.ErrPermissionDenied } + return fileutils.Copy(d.user.Fs, src, dst) case "rename": if !d.user.Perm.Rename { return errors.ErrPermissionDenied } + dst = filepath.Clean("/" + dst) + return d.user.Fs.Rename(src, dst) default: return fmt.Errorf("unsupported action %s: %w", action, errors.ErrInvalidRequestParams) @@ -193,3 +185,35 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request, return errToStatus(err), err }) + +func checkParent(src, dst string) error { + rel, err := filepath.Rel(src, dst) + if err != nil { + return err + } + + rel = filepath.ToSlash(rel) + if !strings.HasPrefix(rel, "../") && rel != ".." && rel != "." { + return errors.ErrSourceIsParent + } + + return nil +} + +func addVersionSuffix(path string, fs afero.Fs) string { + counter := 1 + dir, name := filepath.Split(path) + ext := filepath.Ext(name) + base := strings.TrimSuffix(name, ext) + + for { + if _, err := fs.Stat(path); err != nil { + break + } + renamed := fmt.Sprintf("%s(%d)%s", base, counter, ext) + path = filepath.ToSlash(dir) + renamed + counter++ + } + + return path +} From d9be370e2474b8070fa58db920c9481270cc4a48 Mon Sep 17 00:00:00 2001 From: Ramires Viana <59319979+ramiresviana@users.noreply.github.com> Date: Mon, 20 Jul 2020 17:39:14 +0000 Subject: [PATCH 3/5] fix: missing error message --- frontend/src/api/files.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/api/files.js b/frontend/src/api/files.js index 5942e71a..b3177c9d 100644 --- a/frontend/src/api/files.js +++ b/frontend/src/api/files.js @@ -43,7 +43,7 @@ async function resourceAction (url, method, content) { const res = await fetchURL(`/api/resources${url}`, opts) if (res.status !== 200) { - throw new Error(res.responseText) + throw new Error(await res.text()) } else { return res } From f2d2c1cbf85fba3edffb7b079f121ed3f0bc1e02 Mon Sep 17 00:00:00 2001 From: Ramires Viana <59319979+ramiresviana@users.noreply.github.com> Date: Mon, 20 Jul 2020 18:14:27 +0000 Subject: [PATCH 4/5] fix: drop feedback --- frontend/src/components/files/Listing.vue | 22 ++++++++++++++++------ 1 file changed, 16 insertions(+), 6 deletions(-) diff --git a/frontend/src/components/files/Listing.vue b/frontend/src/components/files/Listing.vue index 6a4eefdb..1d2bf9e8 100644 --- a/frontend/src/components/files/Listing.vue +++ b/frontend/src/components/files/Listing.vue @@ -8,9 +8,7 @@