fix: parent verification on copy

This commit is contained in:
Ramires Viana 2020-07-20 17:38:43 +00:00
parent 34dfb49b71
commit 727c63b98e
2 changed files with 42 additions and 17 deletions

View File

@ -16,4 +16,5 @@ var (
ErrInvalidAuthMethod = errors.New("invalid auth method") ErrInvalidAuthMethod = errors.New("invalid auth method")
ErrPermissionDenied = errors.New("permission denied") ErrPermissionDenied = errors.New("permission denied")
ErrInvalidRequestParams = errors.New("invalid request params") ErrInvalidRequestParams = errors.New("invalid request params")
ErrSourceIsParent = errors.New("source is parent")
) )

View File

@ -10,6 +10,8 @@ import (
"path/filepath" "path/filepath"
"strings" "strings"
"github.com/spf13/afero"
"github.com/filebrowser/filebrowser/v2/errors" "github.com/filebrowser/filebrowser/v2/errors"
"github.com/filebrowser/filebrowser/v2/files" "github.com/filebrowser/filebrowser/v2/files"
"github.com/filebrowser/filebrowser/v2/fileutils" "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") dst := r.URL.Query().Get("destination")
action := r.URL.Query().Get("action") action := r.URL.Query().Get("action")
dst, err := url.QueryUnescape(dst) dst, err := url.QueryUnescape(dst)
if err != nil { if err != nil {
return errToStatus(err), err return errToStatus(err), err
} }
if dst == "/" || src == "/" { if dst == "/" || src == "/" {
return http.StatusForbidden, nil return http.StatusForbidden, nil
} }
if err = checkParent(src, dst); err != nil {
return http.StatusBadRequest, err
}
override := r.URL.Query().Get("override") == "true" override := r.URL.Query().Get("override") == "true"
rename := r.URL.Query().Get("rename") == "true" rename := r.URL.Query().Get("rename") == "true"
if !override && !rename { if !override && !rename {
if _, err = d.user.Fs.Stat(dst); err == nil { if _, err = d.user.Fs.Stat(dst); err == nil {
return http.StatusConflict, nil return http.StatusConflict, nil
} }
} }
if rename { if rename {
counter := 1 dst = addVersionSuffix(dst, d.user.Fs)
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++
}
} }
err = d.RunHook(func() error { err = d.RunHook(func() error {
@ -180,11 +169,14 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request,
if !d.user.Perm.Create { if !d.user.Perm.Create {
return errors.ErrPermissionDenied return errors.ErrPermissionDenied
} }
return fileutils.Copy(d.user.Fs, src, dst) return fileutils.Copy(d.user.Fs, src, dst)
case "rename": case "rename":
if !d.user.Perm.Rename { if !d.user.Perm.Rename {
return errors.ErrPermissionDenied return errors.ErrPermissionDenied
} }
dst = filepath.Clean("/" + dst)
return d.user.Fs.Rename(src, dst) return d.user.Fs.Rename(src, dst)
default: default:
return fmt.Errorf("unsupported action %s: %w", action, errors.ErrInvalidRequestParams) 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 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
}