Skip to content

Commit 727c63b

Browse files
committed
fix: parent verification on copy
1 parent 34dfb49 commit 727c63b

File tree

2 files changed

+42
-17
lines changed

2 files changed

+42
-17
lines changed

errors/errors.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,4 +16,5 @@ var (
1616
ErrInvalidAuthMethod = errors.New("invalid auth method")
1717
ErrPermissionDenied = errors.New("permission denied")
1818
ErrInvalidRequestParams = errors.New("invalid request params")
19+
ErrSourceIsParent = errors.New("source is parent")
1920
)

http/resource.go

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import (
1010
"path/filepath"
1111
"strings"
1212

13+
"github.com/spf13/afero"
14+
1315
"github.com/filebrowser/filebrowser/v2/errors"
1416
"github.com/filebrowser/filebrowser/v2/files"
1517
"github.com/filebrowser/filebrowser/v2/fileutils"
@@ -139,38 +141,25 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request,
139141
dst := r.URL.Query().Get("destination")
140142
action := r.URL.Query().Get("action")
141143
dst, err := url.QueryUnescape(dst)
142-
143144
if err != nil {
144145
return errToStatus(err), err
145146
}
146-
147147
if dst == "/" || src == "/" {
148148
return http.StatusForbidden, nil
149149
}
150+
if err = checkParent(src, dst); err != nil {
151+
return http.StatusBadRequest, err
152+
}
150153

151154
override := r.URL.Query().Get("override") == "true"
152155
rename := r.URL.Query().Get("rename") == "true"
153-
154156
if !override && !rename {
155157
if _, err = d.user.Fs.Stat(dst); err == nil {
156158
return http.StatusConflict, nil
157159
}
158160
}
159-
160161
if rename {
161-
counter := 1
162-
dir, name := filepath.Split(dst)
163-
ext := filepath.Ext(name)
164-
base := strings.TrimSuffix(name, ext)
165-
166-
for {
167-
if _, err = d.user.Fs.Stat(dst); err != nil {
168-
break
169-
}
170-
new := fmt.Sprintf("%s(%d)%s", base, counter, ext)
171-
dst = filepath.ToSlash(dir) + new
172-
counter++
173-
}
162+
dst = addVersionSuffix(dst, d.user.Fs)
174163
}
175164

176165
err = d.RunHook(func() error {
@@ -180,11 +169,14 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request,
180169
if !d.user.Perm.Create {
181170
return errors.ErrPermissionDenied
182171
}
172+
183173
return fileutils.Copy(d.user.Fs, src, dst)
184174
case "rename":
185175
if !d.user.Perm.Rename {
186176
return errors.ErrPermissionDenied
187177
}
178+
dst = filepath.Clean("/" + dst)
179+
188180
return d.user.Fs.Rename(src, dst)
189181
default:
190182
return fmt.Errorf("unsupported action %s: %w", action, errors.ErrInvalidRequestParams)
@@ -193,3 +185,35 @@ var resourcePatchHandler = withUser(func(w http.ResponseWriter, r *http.Request,
193185

194186
return errToStatus(err), err
195187
})
188+
189+
func checkParent(src, dst string) error {
190+
rel, err := filepath.Rel(src, dst)
191+
if err != nil {
192+
return err
193+
}
194+
195+
rel = filepath.ToSlash(rel)
196+
if !strings.HasPrefix(rel, "../") && rel != ".." && rel != "." {
197+
return errors.ErrSourceIsParent
198+
}
199+
200+
return nil
201+
}
202+
203+
func addVersionSuffix(path string, fs afero.Fs) string {
204+
counter := 1
205+
dir, name := filepath.Split(path)
206+
ext := filepath.Ext(name)
207+
base := strings.TrimSuffix(name, ext)
208+
209+
for {
210+
if _, err := fs.Stat(path); err != nil {
211+
break
212+
}
213+
renamed := fmt.Sprintf("%s(%d)%s", base, counter, ext)
214+
path = filepath.ToSlash(dir) + renamed
215+
counter++
216+
}
217+
218+
return path
219+
}

0 commit comments

Comments
 (0)