Skip to content

Commit b374168

Browse files
committed
fix: path traversal with relaxed key in file backend
Signed-off-by: ferhat elmas <elmas.ferhat@gmail.com>
1 parent 39e6e70 commit b374168

2 files changed

Lines changed: 89 additions & 50 deletions

File tree

src/storage/backend/file.ts

Lines changed: 65 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -342,18 +342,17 @@ export class FileBackend implements StorageBackendAdapter {
342342
cacheControl: string
343343
): Promise<string | undefined> {
344344
const uploadId = randomUUID()
345-
const multiPartFolder = this.resolveSecurePath(
346-
path.join('multiparts', uploadId, bucketName, withOptionalVersion(key, version))
347-
)
348-
const multipartFile = this.resolveSecurePath(
349-
path.join(
350-
'multiparts',
351-
uploadId,
352-
bucketName,
353-
withOptionalVersion(key, version),
354-
'metadata.json'
355-
)
356-
)
345+
const multiPartFolder = this.resolveSecureMultipartPath(uploadId, {
346+
bucketName,
347+
key,
348+
version,
349+
})
350+
const multipartFile = this.resolveSecureMultipartPath(uploadId, {
351+
bucketName,
352+
key,
353+
version,
354+
suffix: 'metadata.json',
355+
})
357356
await fsExtra.ensureDir(multiPartFolder)
358357
await fsExtra.writeFile(multipartFile, JSON.stringify({ contentType, cacheControl }))
359358

@@ -368,15 +367,12 @@ export class FileBackend implements StorageBackendAdapter {
368367
partNumber: number,
369368
body: stream.Readable
370369
): Promise<{ ETag?: string }> {
371-
const partPath = this.resolveSecurePath(
372-
path.join(
373-
'multiparts',
374-
uploadId,
375-
bucketName,
376-
withOptionalVersion(key, version),
377-
`part-${partNumber}`
378-
)
379-
)
370+
const partPath = this.resolveSecureMultipartPath(uploadId, {
371+
bucketName,
372+
key,
373+
version,
374+
suffix: `part-${partNumber}`,
375+
})
380376

381377
const writeStream = fsExtra.createWriteStream(partPath)
382378

@@ -404,15 +400,12 @@ export class FileBackend implements StorageBackendAdapter {
404400
}
405401
> {
406402
const partsByEtags = parts.map(async (part) => {
407-
const partFilePath = this.resolveSecurePath(
408-
path.join(
409-
'multiparts',
410-
uploadId,
411-
bucketName,
412-
withOptionalVersion(key, version),
413-
`part-${part.PartNumber}`
414-
)
415-
)
403+
const partFilePath = this.resolveSecureMultipartPath(uploadId, {
404+
bucketName,
405+
key,
406+
version,
407+
suffix: `part-${part.PartNumber}`,
408+
})
416409
const partExists = await fsExtra.pathExists(partFilePath)
417410

418411
if (partExists) {
@@ -432,15 +425,12 @@ export class FileBackend implements StorageBackendAdapter {
432425

433426
const multipartStream = this.mergePartStreams(finalParts)
434427
const metadataContent = await fsExtra.readFile(
435-
this.resolveSecurePath(
436-
path.join(
437-
'multiparts',
438-
uploadId,
439-
bucketName,
440-
withOptionalVersion(key, version),
441-
'metadata.json'
442-
)
443-
),
428+
this.resolveSecureMultipartPath(uploadId, {
429+
bucketName,
430+
key,
431+
version,
432+
suffix: 'metadata.json',
433+
}),
444434
'utf-8'
445435
)
446436

@@ -455,7 +445,7 @@ export class FileBackend implements StorageBackendAdapter {
455445
metadata.cacheControl
456446
)
457447

458-
fsExtra.remove(this.resolveSecurePath(path.join('multiparts', uploadId))).catch(() => {
448+
fsExtra.remove(this.resolveSecureMultipartPath(uploadId)).catch(() => {
459449
// no-op
460450
})
461451

@@ -473,7 +463,7 @@ export class FileBackend implements StorageBackendAdapter {
473463
uploadId: string,
474464
version?: string
475465
): Promise<void> {
476-
const multiPartFolder = this.resolveSecurePath(path.join('multiparts', uploadId))
466+
const multiPartFolder = this.resolveSecureMultipartPath(uploadId)
477467

478468
await fsExtra.remove(multiPartFolder)
479469

@@ -495,15 +485,12 @@ export class FileBackend implements StorageBackendAdapter {
495485
sourceVersion?: string,
496486
rangeBytes?: { fromByte: number; toByte: number }
497487
): Promise<{ eTag?: string; lastModified?: Date }> {
498-
const partFilePath = this.resolveSecurePath(
499-
path.join(
500-
'multiparts',
501-
UploadId,
502-
storageS3Bucket,
503-
withOptionalVersion(key, version),
504-
`part-${PartNumber}`
505-
)
506-
)
488+
const partFilePath = this.resolveSecureMultipartPath(UploadId, {
489+
bucketName: storageS3Bucket,
490+
key,
491+
version,
492+
suffix: `part-${PartNumber}`,
493+
})
507494
const sourceFilePath = this.resolveSecurePath(
508495
`${storageS3Bucket}/${withOptionalVersion(sourceKey, sourceVersion)}`
509496
)
@@ -692,6 +679,34 @@ export class FileBackend implements StorageBackendAdapter {
692679
return normalizedPath
693680
}
694681

682+
private resolveSecureMultipartPath(
683+
uploadId: string,
684+
options?: {
685+
bucketName?: string
686+
key?: string
687+
version?: string
688+
suffix?: string
689+
}
690+
): string {
691+
// Intentionally avoid path.join for attacker-controlled segments so dot segments remain visible
692+
// to resolveSecurePath instead of being normalized away beforehand.
693+
let relativePath = `multiparts/${uploadId}`
694+
695+
if (typeof options?.bucketName === 'string') {
696+
relativePath += `/${options.bucketName}`
697+
}
698+
699+
if (typeof options?.key === 'string') {
700+
relativePath += `/${withOptionalVersion(options.key, options.version)}`
701+
}
702+
703+
if (typeof options?.suffix === 'string') {
704+
relativePath += `/${options.suffix}`
705+
}
706+
707+
return this.resolveSecurePath(relativePath)
708+
}
709+
695710
private async etag(file: string, stats: fs.Stats): Promise<string> {
696711
if (this.etagAlgorithm === 'md5') {
697712
const checksum = await this.computeMd5(file)

src/test/file-backend.test.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -314,6 +314,22 @@ describe('FileBackend traversal protection', () => {
314314
})
315315
})
316316

317+
it('rejects multipart keys that only rebase within the storage root', async () => {
318+
const traversalKey = '../multipart-rebased.txt'
319+
320+
await expect(
321+
backend.createMultiPartUpload('bucket', traversalKey, 'v1', 'text/plain', 'no-cache')
322+
).rejects.toMatchObject({
323+
code: 'InvalidKey',
324+
})
325+
326+
await expect(
327+
backend.uploadPart('bucket', traversalKey, 'v1', 'upload-id', 1, Readable.from('escape-part'))
328+
).rejects.toMatchObject({
329+
code: 'InvalidKey',
330+
})
331+
})
332+
317333
it('rejects traversal key in object operations with InvalidKey', async () => {
318334
const traversalKey = `${'../'.repeat(20)}tmp/${escapePrefix}/object-escape.txt`
319335

@@ -408,6 +424,14 @@ describe('FileBackend traversal protection', () => {
408424
code: 'InvalidKey',
409425
})
410426
})
427+
428+
it('rejects multipart upload ids that would be normalized to sibling in-root paths', async () => {
429+
await expect(
430+
backend.abortMultipartUpload('bucket', 'key', '../upload-id')
431+
).rejects.toMatchObject({
432+
code: 'InvalidKey',
433+
})
434+
})
411435
})
412436

413437
describe('FileBackend lastModified', () => {

0 commit comments

Comments
 (0)