fix: webdav rename missing validation
Massimo Melina committed
Jul 2, 2026 at 12:07 UTC
e26c4d10fdd428129e1bee35a43fa79880f1877b
3 files changed
+47
-32
src/frontEndApis.ts
+25
-28
@@ -201,35 +201,32 @@ export async function moveFiles(uri_from: any, uri_to: any, ctx: Koa.Context, ov
201
export async function requestedRename(node: VfsNode | undefined, newName: string, ctx: Koa.Context) {
202
if (!node)
203
throw new ApiError(HTTP_NOT_FOUND)
204
+ // requestedRename is exported, so keep disk rename confinement here even when callers pre-validate
205
+ if (!isValidFileName(newName))
206
+ throw new ApiError(HTTP_BAD_REQUEST)
207
if (statusCodeForMissingPerm(node, 'can_delete', ctx))
205
- return new ApiError(ctx.status)
206
- try {
207
- if (node.name) // virtual name = virtual rename
208
- node.name = newName
209
- else {
210
- if (!node.source)
211
- throw new ApiError(HTTP_FAILED_DEPENDENCY)
212
- const destNode = await urlToNode(pathEncode(newName), ctx, node.parent)
213
- if (destNode && statusCodeForMissingPerm(destNode, 'can_delete', ctx)) // if destination exists, you need delete permission
214
- return new ApiError(ctx.status)
215
- try {
216
- const destSource = join(dirname(node.source), newName)
217
- await rename(node.source, destSource)
218
- await moveStoredFileAttrs(node.source, destSource)
219
- getCommentFor(node.source).then(c => {
220
- if (!c) return
221
- void setCommentFor(node.source!, '')
222
- void setCommentFor(destSource, c)
223
- })
224
- return {}
225
- }
226
- catch (e: any) {
227
- return new ApiError(HTTP_SERVER_ERROR, e)
228
- }
208
+ throw new ApiError(ctx.status)
209
+ if (node.name) // virtual name = virtual rename
210
+ node.name = newName
211
+ else {
212
+ if (!node.source)
213
+ throw new ApiError(HTTP_FAILED_DEPENDENCY)
214
+ const destNode = await urlToNode(pathEncode(newName), ctx, node.parent)
215
+ if (destNode && statusCodeForMissingPerm(destNode, 'can_delete', ctx)) // if destination exists, you need delete permission
216
+ throw new ApiError(ctx.status)
217
+ try {
218
+ const destSource = join(dirname(node.source), newName)
219
+ await rename(node.source, destSource)
220
+ await moveStoredFileAttrs(node.source, destSource)
221
+ getCommentFor(node.source).then(c => {
222
+ if (!c) return
223
+ void setCommentFor(node.source!, '')
224
+ void setCommentFor(destSource, c)
225
+ })
226
+ return {}
227
+ }
228
+ catch (e: any) {
229
+ throw new ApiError(HTTP_SERVER_ERROR, e)
230
}
230
- return
231
- }
232
- catch (e: any) {
233
- throw new ApiError(HTTP_SERVER_ERROR, e)
231
}
232
}
src/webdav.ts
+1
-4
@@ -217,10 +217,7 @@ export const webdav: Koa.Middleware = async (ctx, next) => {
217
if (dirname(path) === dirname(dest)) // rename case. `path` is is encoded, so we test before decoding `dest`
218
try {
219
// decode the single path segment so reserved chars like %2C become their real name on rename
220
- const newName = safeDecodeURIComponent(basename(dest), '')
221
- if (!newName)
222
- return ctx.status = HTTP_BAD_REQUEST
223
- await requestedRename(node, newName, ctx)
220
+ await requestedRename(node, safeDecodeURIComponent(basename(dest), ''), ctx)
221
releaseWebdavLock(path) // RFC 4918 says MOVE must not carry locks to destination, so clear source lock on success
222
return ctx.status = HTTP_CREATED
223
}
tests/test.ts
+21
@@ -652,6 +652,27 @@ describe('webdav', () => {
652
}
653
}
654
})
655
+ test('webdav.move rename cannot traverse out of root', async () => {
656
+ const name = `wd-move-trav-${randomId(6)}.txt`
657
+ const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`
658
+ const escapedName = `wd-escaped-${randomId(6)}.txt`
659
+ const traversal = `../../${escapedName}` // climbs above the upload node's source
660
+ // encode as a single path segment so dirname(dest) still matches dirname(path) and we hit the rename branch
661
+ const destination = `${BASE_URL}${UPLOAD_ROOT}${UPLOAD_DIR}/${encodeURIComponent(traversal)}`
662
+ const escapedDiskPath = resolve(ROOT, UPLOAD_DIR, traversal)
663
+ let destPath = ''
664
+ try {
665
+ destPath = await webdavUpload(uri, x => x?.uri === uri, 'test')()
666
+ await req(uri, 400, { method: 'MOVE', auth, jar, headers: { destination, overwrite: 'F', 'user-agent': WEBDAV_UA } })()
667
+ if (await access(escapedDiskPath).then(() => true, () => false))
668
+ throw "file escaped the VFS root"
669
+ await req(uri, 200, { auth })() // source must still be there, untouched
670
+ }
671
+ finally {
672
+ await rmAny(escapedDiskPath)
673
+ await rmAny(destPath)
674
+ }
675
+ })
676
test('webdav.proppatch accepts dead properties as no-op', async () => {
677
const name = `wd-proppatch-${randomId(6)}.txt`
678
const uri = `${UPLOAD_ROOT}${UPLOAD_DIR}/${name}`