Skip to content

Commit c7c0cfa

Browse files
authored
fix(fsmanage): improve path validation (#2437)
* fix(fsmanage): improve path validation in FsMove, FsCopy, and FsRemove functions
1 parent d3a6b06 commit c7c0cfa

1 file changed

Lines changed: 33 additions & 25 deletions

File tree

server/handles/fsmanage.go

Lines changed: 33 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import (
1717
"github.com/OpenListTeam/OpenList/v4/server/common"
1818
"github.com/gin-gonic/gin"
1919
"github.com/pkg/errors"
20-
log "github.com/sirupsen/logrus"
2120
)
2221

2322
type MkdirOrLinkReq struct {
@@ -110,15 +109,15 @@ func FsMove(c *gin.Context) {
110109
common.ErrorResp(c, errs.PermissionDenied, 403)
111110
return
112111
}
113-
114-
validPaths := make([]string, 0, len(req.Names))
115-
for _, name := range req.Names {
112+
if !strings.HasSuffix(srcDir, "/") {
113+
srcDir += "/"
114+
}
115+
for i, name := range req.Names {
116116
// ensure req.Names is not a relative path
117-
srcPath := stdpath.Join(req.SrcDir, name)
118-
srcPath, err = user.JoinPath(srcPath)
119-
if err != nil {
120-
common.ErrorResp(c, err, 403)
121-
return
117+
srcPath := stdpath.Join(srcDir, name)
118+
if !strings.HasPrefix(srcPath+"/", srcDir) {
119+
req.Names[i] = ""
120+
continue
122121
}
123122
if !req.Overwrite {
124123
base := stdpath.Base(srcPath)
@@ -135,14 +134,17 @@ func FsMove(c *gin.Context) {
135134
}
136135
}
137136
}
138-
validPaths = append(validPaths, srcPath)
137+
req.Names[i] = srcPath
139138
}
140139

141140
// Create all tasks immediately without any synchronous validation
142141
// All validation will be done asynchronously in the background
143142
var addedTasks []task.TaskExtensionInfo
144-
for i, p := range validPaths {
145-
t, err := fs.Move(c.Request.Context(), p, dstDir, len(validPaths) > i+1)
143+
for i, p := range req.Names {
144+
if p == "" {
145+
continue
146+
}
147+
t, err := fs.Move(c.Request.Context(), p, dstDir, len(req.Names) > i+1)
146148
if t != nil {
147149
addedTasks = append(addedTasks, t)
148150
}
@@ -210,14 +212,15 @@ func FsCopy(c *gin.Context) {
210212
return
211213
}
212214

213-
validPaths := make([]string, 0, len(req.Names))
214-
for _, name := range req.Names {
215+
if !strings.HasSuffix(srcDir, "/") {
216+
srcDir += "/"
217+
}
218+
for i, name := range req.Names {
215219
// ensure req.Names is not a relative path
216-
srcPath := stdpath.Join(req.SrcDir, name)
217-
srcPath, err = user.JoinPath(srcPath)
218-
if err != nil {
219-
common.ErrorResp(c, err, 403)
220-
return
220+
srcPath := stdpath.Join(srcDir, name)
221+
if !strings.HasPrefix(srcPath+"/", srcDir) {
222+
req.Names[i] = ""
223+
continue
221224
}
222225
if !req.Overwrite {
223226
base := stdpath.Base(srcPath)
@@ -234,18 +237,21 @@ func FsCopy(c *gin.Context) {
234237
}
235238
}
236239
}
237-
validPaths = append(validPaths, srcPath)
240+
req.Names[i] = srcPath
238241
}
239242

240243
// Create all tasks immediately without any synchronous validation
241244
// All validation will be done asynchronously in the background
242245
var addedTasks []task.TaskExtensionInfo
243-
for i, p := range validPaths {
246+
for i, p := range req.Names {
247+
if p == "" {
248+
continue
249+
}
244250
var t task.TaskExtensionInfo
245251
if req.Merge {
246-
t, err = fs.Merge(c.Request.Context(), p, dstDir, len(validPaths) > i+1)
252+
t, err = fs.Merge(c.Request.Context(), p, dstDir, len(req.Names) > i+1)
247253
} else {
248-
t, err = fs.Copy(c.Request.Context(), p, dstDir, len(validPaths) > i+1)
254+
t, err = fs.Copy(c.Request.Context(), p, dstDir, len(req.Names) > i+1)
249255
}
250256
if t != nil {
251257
addedTasks = append(addedTasks, t)
@@ -362,10 +368,12 @@ func FsRemove(c *gin.Context) {
362368
common.ErrorResp(c, errs.PermissionDenied, 403)
363369
return
364370
}
371+
if !strings.HasSuffix(reqPath, "/") {
372+
reqPath += "/"
373+
}
365374
for i, name := range req.Names {
366375
fullPath := stdpath.Join(reqPath, name)
367-
if !strings.HasPrefix(fullPath+"/", reqPath+"/") {
368-
log.Warnf("FsRemove: path traversal attempt skipped: %s (dir: %s)\n", name, req.Dir)
376+
if !strings.HasPrefix(fullPath+"/", reqPath) {
369377
req.Names[i] = ""
370378
continue
371379
}

0 commit comments

Comments
 (0)