mirror of
https://github.com/docmost/docmost.git
synced 2026-05-07 06:23:06 +08:00
fix: tighten local storage access
This commit is contained in:
@@ -53,6 +53,7 @@ import { EnvironmentService } from '../../integrations/environment/environment.s
|
|||||||
import { TokenService } from '../auth/services/token.service';
|
import { TokenService } from '../auth/services/token.service';
|
||||||
import { JwtAttachmentPayload, JwtType } from '../auth/dto/jwt-payload';
|
import { JwtAttachmentPayload, JwtType } from '../auth/dto/jwt-payload';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
|
import { sanitize } from 'sanitize-filename-ts';
|
||||||
import { AttachmentInfoDto, RemoveIconDto } from './dto/attachment.dto';
|
import { AttachmentInfoDto, RemoveIconDto } from './dto/attachment.dto';
|
||||||
import { PageAccessService } from '../page/page-access/page-access.service';
|
import { PageAccessService } from '../page/page-access/page-access.service';
|
||||||
import { AuditEvent, AuditResource } from '../../common/events/audit-events';
|
import { AuditEvent, AuditResource } from '../../common/events/audit-events';
|
||||||
@@ -356,6 +357,10 @@ export class AttachmentController {
|
|||||||
throw new BadRequestException('Invalid image attachment type');
|
throw new BadRequestException('Invalid image attachment type');
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (!fileName || sanitize(fileName) !== fileName) {
|
||||||
|
throw new BadRequestException('Invalid file name');
|
||||||
|
}
|
||||||
|
|
||||||
const filenameWithoutExt = path.basename(fileName, path.extname(fileName));
|
const filenameWithoutExt = path.basename(fileName, path.extname(fileName));
|
||||||
if (!isValidUUID(filenameWithoutExt)) {
|
if (!isValidUUID(filenameWithoutExt)) {
|
||||||
throw new BadRequestException('Invalid file id');
|
throw new BadRequestException('Invalid file id');
|
||||||
|
|||||||
@@ -0,0 +1,67 @@
|
|||||||
|
import { resolve, sep } from 'path';
|
||||||
|
import { LocalDriver } from './local.driver';
|
||||||
|
|
||||||
|
type FullPath = (filePath: string) => string;
|
||||||
|
|
||||||
|
describe('LocalDriver._fullPath', () => {
|
||||||
|
const ROOT = resolve('/data/storage');
|
||||||
|
const driver = new LocalDriver({ storagePath: ROOT });
|
||||||
|
const fullPath = ((driver as any)._fullPath as FullPath).bind(driver);
|
||||||
|
|
||||||
|
describe('legitimate inputs (behavior preserved)', () => {
|
||||||
|
it.each([
|
||||||
|
['workspace-id/avatars/uuid.png', `${ROOT}${sep}workspace-id${sep}avatars${sep}uuid.png`],
|
||||||
|
['workspace-id/files/uuid/file.pdf', `${ROOT}${sep}workspace-id${sep}files${sep}uuid${sep}file.pdf`],
|
||||||
|
['a/b/c/d/e.bin', `${ROOT}${sep}a${sep}b${sep}c${sep}d${sep}e.bin`],
|
||||||
|
['', ROOT],
|
||||||
|
['.', ROOT],
|
||||||
|
['./x/y.png', `${ROOT}${sep}x${sep}y.png`],
|
||||||
|
['a//b', `${ROOT}${sep}a${sep}b`],
|
||||||
|
['a/b/../c', `${ROOT}${sep}a${sep}c`],
|
||||||
|
])('resolves %j to %j', (input, expected) => {
|
||||||
|
expect(fullPath(input)).toBe(expected);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('traversal rejected', () => {
|
||||||
|
it.each([
|
||||||
|
'../etc/passwd',
|
||||||
|
'../../../etc/passwd',
|
||||||
|
'workspace/../../../etc/passwd',
|
||||||
|
'..',
|
||||||
|
'../..',
|
||||||
|
'a/../../..',
|
||||||
|
])('throws for %j', (input) => {
|
||||||
|
expect(() => fullPath(input)).toThrow('Invalid file path');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('absolute path rejected', () => {
|
||||||
|
it.each([
|
||||||
|
'/etc/passwd',
|
||||||
|
'/root/.ssh/id_rsa',
|
||||||
|
sep + 'absolute',
|
||||||
|
])('throws for %j', (input) => {
|
||||||
|
expect(() => fullPath(input)).toThrow('Invalid file path');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('prefix-confusion rejected', () => {
|
||||||
|
it('rejects a sibling directory whose name starts with the storage root', () => {
|
||||||
|
const siblingDriver = new LocalDriver({ storagePath: '/data/storage' });
|
||||||
|
const siblingFullPath = ((siblingDriver as any)._fullPath as FullPath).bind(siblingDriver);
|
||||||
|
// Attempt to reach /data/storage-evil/secret by traversal:
|
||||||
|
// resolve('/data/storage', '../storage-evil/secret') === '/data/storage-evil/secret'
|
||||||
|
// Without the `+ sep` guard, a startsWith check would match.
|
||||||
|
expect(() => siblingFullPath('../storage-evil/secret')).toThrow('Invalid file path');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('storage root itself', () => {
|
||||||
|
it('accepts the root when input resolves to it', () => {
|
||||||
|
expect(fullPath('')).toBe(ROOT);
|
||||||
|
expect(fullPath('.')).toBe(ROOT);
|
||||||
|
expect(fullPath('a/..')).toBe(ROOT);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -3,7 +3,7 @@ import {
|
|||||||
LocalStorageConfig,
|
LocalStorageConfig,
|
||||||
StorageOption,
|
StorageOption,
|
||||||
} from '../interfaces';
|
} from '../interfaces';
|
||||||
import { join, dirname } from 'path';
|
import { dirname, resolve, sep } from 'path';
|
||||||
import * as fs from 'fs-extra';
|
import * as fs from 'fs-extra';
|
||||||
import { Readable } from 'stream';
|
import { Readable } from 'stream';
|
||||||
import { createReadStream, createWriteStream } from 'node:fs';
|
import { createReadStream, createWriteStream } from 'node:fs';
|
||||||
@@ -17,7 +17,12 @@ export class LocalDriver implements StorageDriver {
|
|||||||
}
|
}
|
||||||
|
|
||||||
private _fullPath(filePath: string): string {
|
private _fullPath(filePath: string): string {
|
||||||
return join(this.config.storagePath, filePath);
|
const storageRoot = resolve(this.config.storagePath);
|
||||||
|
const fullPath = resolve(storageRoot, filePath);
|
||||||
|
if (fullPath !== storageRoot && !fullPath.startsWith(storageRoot + sep)) {
|
||||||
|
throw new Error('Invalid file path');
|
||||||
|
}
|
||||||
|
return fullPath;
|
||||||
}
|
}
|
||||||
|
|
||||||
async upload(filePath: string, file: Buffer | Readable): Promise<void> {
|
async upload(filePath: string, file: Buffer | Readable): Promise<void> {
|
||||||
|
|||||||
Reference in New Issue
Block a user